[RESEND][PATCH 6/7] xfs: Remove code handling bio_alloc failure with __GFP_WAIT

[RESEND][PATCH 6/7] xfs: Remove code handling bio_alloc failure with __GFP_WAIT

[Nikanth Karthikesan]

Resending as I accidentally missed Jens earlier.

Jens, can you merge this as well.

Thanks
Nikanth

Remove code handling bio_alloc failure with __GFP_WAIT.
GFP_NOIO implies __GFP_WAIT.

Signed-off-by: Nikanth Karthikesan <knikanth@suse.de>

---

diff --git a/fs/xfs/linux-2.6/xfs_aops.c b/fs/xfs/linux-2.6/xfs_aops.c
index 7ec89fc..fb4f516 100644
--- a/fs/xfs/linux-2.6/xfs_aops.c
+++ b/fs/xfs/linux-2.6/xfs_aops.c
@@ -421,10 +421,7 @@ xfs_alloc_ioend_bio(
 	struct bio		*bio;
 	int			nvecs = bio_get_nr_vecs(bh->b_bdev);
 
-	do {
-		bio = bio_alloc(GFP_NOIO, nvecs);
-		nvecs >>= 1;
-	} while (!bio);
+	bio = bio_alloc(GFP_NOIO, nvecs);
 
 	ASSERT(bio->bi_private == NULL);
 	bio->bi_sector = bh->b_blocknr * (bh->b_size >> 9);


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [xfs-masters] [RESEND][PATCH 6/7] xfs: Remove code handling bio_alloc failure with __GFP_WAIT

[Dave Chinner]

On Wed, Apr 15, 2009 at 04:09:30PM +0530, Nikanth Karthikesan wrote:
> Resending as I accidentally missed Jens earlier.
> 
> Jens, can you merge this as well.
> 
> Thanks
> Nikanth
> 
> Remove code handling bio_alloc failure with __GFP_WAIT.
> GFP_NOIO implies __GFP_WAIT.

Not sure that is right. The intent of the code is that if we can't
get a large bio immediately, try a smaller one which is more likely
to succeed when we are under memory pressure. i.e. we will get IO
moving faster than if we waited for a maximally sized biovec to be
allocated.

IOWs, I don't think __GFP_WAIT is implied by this code, regardless
of what GFP_NOIO actually means now. The same code fragment can be
found in NILFS, and it uses GFP_NOWAIT, not GFP_NOIO. I suspect that
this is what this XFS code should be changed to use to retain the
original intent of the code....

Cheers,

Dave.
-- 
Dave Chinner
dgc@evostor.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [xfs-masters] [RESEND][PATCH 6/7] xfs: Remove code handling bio_alloc failure with __GFP_WAIT

[Nikanth Karthikesan]

On Monday 20 April 2009 06:32:37 Dave Chinner wrote:
> On Wed, Apr 15, 2009 at 04:09:30PM +0530, Nikanth Karthikesan wrote:
> > Resending as I accidentally missed Jens earlier.
> >
> > Jens, can you merge this as well.
> >
> > Thanks
> > Nikanth
> >
> > Remove code handling bio_alloc failure with __GFP_WAIT.
> > GFP_NOIO implies __GFP_WAIT.
>
> Not sure that is right. The intent of the code is that if we can't
> get a large bio immediately, try a smaller one which is more likely
> to succeed when we are under memory pressure. i.e. we will get IO
> moving faster than if we waited for a maximally sized biovec to be
> allocated.
>
> IOWs, I don't think __GFP_WAIT is implied by this code, regardless
> of what GFP_NOIO actually means now. The same code fragment can be
> found in NILFS, and it uses GFP_NOWAIT, not GFP_NOIO. I suspect that
> this is what this XFS code should be changed to use to retain the
> original intent of the code....
>

Yes, if that is the intent, it needs to be changed as GFP_NOWAIT. Otherwise 
the first attempt to get a larger bio itself would return only after it 
succeeds, which makes the logic useless.

Thanks
Nikanth

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] xfs: fix xfs_alloc_ioend_bio code to try and get atleast a smaller bio

[Nikanth Karthikesan]

The intent of the code in xfs_alloc_ioend_bio() is that if we can't get a
large bio immediately, try a smaller one which is more likely to succeed when
we are under memory pressure. i.e. we will get IO moving faster than if we
waited for a maximally sized biovec to be allocated. But GFP_NOIO implies
__GFP_WAIT which would return only if it can allocate the bio. So the first
attempt to get a larger bio itself would return only after it succeeds, which
makes the logic useless.

Change it to try with GFP_NOWAIT for a bio. If not possible wait for it.

See http://oss.sgi.com/archives/xfs-masters/2009-04/msg00027.html for the
discussion.

Signed-off-by: Nikanth Karthikesan <knikanth@suse.de>

---

diff --git a/fs/xfs/linux-2.6/xfs_aops.c b/fs/xfs/linux-2.6/xfs_aops.c
index 7ec89fc..002ce7f 100644
--- a/fs/xfs/linux-2.6/xfs_aops.c
+++ b/fs/xfs/linux-2.6/xfs_aops.c
@@ -419,12 +419,15 @@ xfs_alloc_ioend_bio(
 	struct buffer_head	*bh)
 {
 	struct bio		*bio;
-	int			nvecs = bio_get_nr_vecs(bh->b_bdev);
+	int			nr_iovecs = bio_get_nr_vecs(bh->b_bdev);
+	int			nvecs = nr_iovecs;
 
 	do {
-		bio = bio_alloc(GFP_NOIO, nvecs);
-		nvecs >>= 1;
-	} while (!bio);
+		bio = bio_alloc(GFP_NOWAIT, nvecs);
+	} while (!bio && (nvecs >>= 1));
+
+	if (unlikely(!bio))
+		bio = bio_alloc(GFP_NOIO, nr_iovecs);
 
 	ASSERT(bio->bi_private == NULL);
 	bio->bi_sector = bh->b_blocknr * (bh->b_size >> 9);

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs