linux-kernel - Re: [PATCH 04/12] DRBD: request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <807b3a220904080316w54426d86jc4ba67ce9275edb8@mail.gmail.com>
Date:	Wed, 8 Apr 2009 15:46:42 +0530
From:	Nikanth K <nikanth@...il.com>
To:	Philipp Reisner <philipp.reisner@...bit.com>
Cc:	linux-kernel@...r.kernel.org, gregkh@...e.de,
	Nikanth Karthikesan <knikanth@...e.de>
Subject: Re: [PATCH 04/12] DRBD: request

On Mon, Mar 23, 2009 at 9:17 PM, Philipp Reisner
<philipp.reisner@...bit.com> wrote:
<snip>

> +
> +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.


<snip>



> +#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 defined
only 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)
<snip>

> +
> +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 the
bio. So this could be changed to return void.

<snip>

> +
> +int drbd_make_request_26(struct request_queue *q, struct bio *bio)

Always returns zero again.

<snip>

> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,28)
> +                               bio_split_pool,
> +#endif

Could be removed for mainline inclusion.


Thanks
Nikanth