All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Moyer <jmoyer@redhat.com>
To: David Dillow <dillowda@ornl.gov>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: fs/direct-io.c: don't try to allocate more than BIO_MAX_PAGES in a bio
Date: Fri, 14 Jan 2011 09:44:33 -0500	[thread overview]
Message-ID: <x498vynbl4e.fsf@segfault.boston.devel.redhat.com> (raw)
In-Reply-To: <1294977946.21790.20.camel@obelisk.thedillows.org> (David Dillow's message of "Thu, 13 Jan 2011 23:05:46 -0500")

David Dillow <dillowda@ornl.gov> writes:

> When using devices that support max_segments > BIO_MAX_PAGES (256),
> direct IO tries to allocate a bio with more pages than allowed, which
> leads to an oops in dio_bio_alloc(). Clamp that request to the supported
> maximum.
>
> Signed-off-by: David Dillow <dillowda@ornl.gov>
> --
> dio_bio_alloc() doesn't check the result of bio_alloc(), so it
> dereferences a NULL pointer. bio_alloc(GFP_KERNEL, ...) doesn't fail
> unless it gets called for an invalid number of pages, so it seems a bit
> like overkill to check for failure in dio_bio_alloc(), though it would
> have saved me some time tracking this down.

We should always be checking return values.  Mind respinning the patch
to include that?

> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 85882f6..9eb0553 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -583,6 +583,7 @@ static int dio_new_bio(struct dio *dio, sector_t start_sector)
>  		goto out;
>  	sector = start_sector << (dio->blkbits - 9);
>  	nr_pages = min(dio->pages_in_io, bio_get_nr_vecs(dio->map_bh.b_bdev));
> +	nr_pages = min(nr_pages, BIO_MAX_PAGES);

This looks fine.  Thanks for tracking it down!

Cheers,
Jeff

  reply	other threads:[~2011-01-14 14:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-14  4:05 fs/direct-io.c: don't try to allocate more than BIO_MAX_PAGES in a bio David Dillow
2011-01-14 14:44 ` Jeff Moyer [this message]
2011-01-14 16:46   ` [PATCH v2 alt 1] " David Dillow
2011-01-14 16:46   ` [PATCH v2 alt 2] " David Dillow
2011-01-17 18:28     ` Jeff Moyer
2011-01-18  3:00       ` [PATCH v3] " David Dillow
2011-01-19  0:53         ` Andrew Morton
2011-01-19  2:34           ` David Dillow
2011-01-19 15:53             ` Boaz Harrosh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=x498vynbl4e.fsf@segfault.boston.devel.redhat.com \
    --to=jmoyer@redhat.com \
    --cc=dillowda@ornl.gov \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.