On Mon, Mar 23, 2009 at 9:17 PM, Philipp Reisner wrote: > +> +static inline struct drbd_request *drbd_req_new(struct drbd_conf *mdev,> +       struct bio *bio_src)> +{> +       struct bio *bio;> +       struct drbd_request *req => +               mempool_alloc(drbd_request_mempool, GFP_NOIO);> +       if (likely(req)) {> +               bio = bio_clone(bio_src, GFP_NOIO); /* XXX cannot fail?? */ I think, bio_clone can fail. > +#define OVERLAPS overlaps(sector, size, i->sector, i->size)> +               slot = tl_hash_slot(mdev, sector);> +               hlist_for_each_entry(i, n, slot, colision) {> +                       if (OVERLAPS) {> +                               ALERT("LOGIC BUG: completed: %p %llus +%u; "> +                                     "other: %p %llus +%u\n",> +                                     req, (unsigned long long)sector, size,> +                                     i, (unsigned long long)i->sector, i->size);> +                       }> +               }> +> +               /* maybe "wake" those conflicting epoch entries> +                * that wait for this request to finish.> +                *> +                * currently, there can be only _one_ such ee> +                * (well, or some more, which would be pending> +                * DiscardAck not yet sent by the asender...),> +                * since we block the receiver thread upon the> +                * first conflict detection, which will wait on> +                * misc_wait.  maybe we want to assert that?> +                *> +                * anyways, if we found one,> +                * we just have to do a wake_up.  */> +#undef OVERLAPS> +#define OVERLAPS overlaps(sector, size, e->sector, e->size) These #defines can be removed? Or Can be parameterized and definedonly once. They are being defined and undefined repeatedly. > +               slot = ee_hash_slot(mdev, req->sector);> +               hlist_for_each_entry(e, n, slot, colision) {> +                       if (OVERLAPS) {> +                               wake_up(&mdev->misc_wait);> +                               break;> +                       }> +               }> +       }> +#undef OVERLAPS> +}> +> +static void _complete_master_bio(struct drbd_conf *mdev,> +       struct drbd_request *req, int error) > +> +STATIC int drbd_make_request_common(struct drbd_conf *mdev, struct bio *bio)> +{> +       const int rw = bio_rw(bio);> +       const int size = bio->bi_size;> +       const sector_t sector = bio->bi_sector;> +       struct drbd_barrier *b = NULL;> +       struct drbd_request *req;> +       int local, remote;> +       int err = -EIO;> +> +       /* allocate outside of all locks; */> +       req = drbd_req_new(mdev, bio);> +       if (!req) {> +               dec_ap_bio(mdev);> +               /* only pass the error to the upper layers.> +                * if user cannot handle io errors, thats not our business. */> +               ERR("could not kmalloc() req\n");> +               bio_endio(bio, -ENOMEM);> +               return 0; Seems to be always returning zero and passing the error through thebio. So this could be changed to return void. > +> +int drbd_make_request_26(struct request_queue *q, struct bio *bio) Always returns zero again. > +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,28)> +                               bio_split_pool,> +#endif Could be removed for mainline inclusion. ThanksNikanth{.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I