Patchwork disallow FS recursion from sb_issue_discard allocation

login
register
mail settings
Submitter Mike Snitzer
Date July 6, 2010, 4:11 p.m.
Message ID <20100706161155.GA24461@redhat.com>
Download mbox | patch
Permalink /patch/58046/
State New
Headers show

Comments

Mike Snitzer - July 6, 2010, 4:11 p.m.
Filesystems can call sb_issue_discard on a memory reclaim path
(e.g. ext4 calls sb_issue_discard during journal commit).

Use GFP_NOFS in sb_issue_discard to avoid recursing back into the FS.

Reported-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 include/linux/blkdev.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o - July 27, 2010, 1:44 p.m.
On Tue, Jul 06, 2010 at 12:11:56PM -0400, Mike Snitzer wrote:
> Filesystems can call sb_issue_discard on a memory reclaim path
> (e.g. ext4 calls sb_issue_discard during journal commit).
> 
> Use GFP_NOFS in sb_issue_discard to avoid recursing back into the FS.
> 
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>

Hi Jens,

I never saw an ack from you on this patch.  Are you ok with it, and
have you grabbed it for your tree?  Do you want me to include this in
the ext4 tree, even though it's a patch to include/linux/blkdev.h?

Thanks,

						- Ted


> ---
>  include/linux/blkdev.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index baf5258..dbb510c 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -933,7 +933,7 @@ static inline int sb_issue_discard(struct super_block *sb,
>  {
>  	block <<= (sb->s_blocksize_bits - 9);
>  	nr_blocks <<= (sb->s_blocksize_bits - 9);
> -	return blkdev_issue_discard(sb->s_bdev, block, nr_blocks, GFP_KERNEL,
> +	return blkdev_issue_discard(sb->s_bdev, block, nr_blocks, GFP_NOFS,
>  				   BLKDEV_IFL_WAIT | BLKDEV_IFL_BARRIER);
>  }
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Snitzer - July 27, 2010, 3:33 p.m.
On Tue, Jul 27 2010 at  9:44am -0400,
Ted Ts'o <tytso@mit.edu> wrote:

> On Tue, Jul 06, 2010 at 12:11:56PM -0400, Mike Snitzer wrote:
> > Filesystems can call sb_issue_discard on a memory reclaim path
> > (e.g. ext4 calls sb_issue_discard during journal commit).
> > 
> > Use GFP_NOFS in sb_issue_discard to avoid recursing back into the FS.
> > 
> > Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> > Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> 
> Hi Jens,
> 
> I never saw an ack from you on this patch.  Are you ok with it, and
> have you grabbed it for your tree?  Do you want me to include this in
> the ext4 tree, even though it's a patch to include/linux/blkdev.h?

Hi Ted,

Thanks for following up on this.  In my experience, Jens is more apt to
pick up a patch if it gets explicitly 'Acked-by' other stake-holders
(especially when a patch is motivated by another subsystem, in this case
the proposed block change addresses a problem unique to fs/ext4).

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jens Axboe - July 28, 2010, 6:12 p.m.
On 07/27/2010 05:33 PM, Mike Snitzer wrote:
> On Tue, Jul 27 2010 at  9:44am -0400,
> Ted Ts'o <tytso@mit.edu> wrote:
> 
>> On Tue, Jul 06, 2010 at 12:11:56PM -0400, Mike Snitzer wrote:
>>> Filesystems can call sb_issue_discard on a memory reclaim path
>>> (e.g. ext4 calls sb_issue_discard during journal commit).
>>>
>>> Use GFP_NOFS in sb_issue_discard to avoid recursing back into the FS.
>>>
>>> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
>>> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
>>
>> Hi Jens,
>>
>> I never saw an ack from you on this patch.  Are you ok with it, and
>> have you grabbed it for your tree?  Do you want me to include this in
>> the ext4 tree, even though it's a patch to include/linux/blkdev.h?
> 
> Hi Ted,
> 
> Thanks for following up on this.  In my experience, Jens is more apt to
> pick up a patch if it gets explicitly 'Acked-by' other stake-holders
> (especially when a patch is motivated by another subsystem, in this case
> the proposed block change addresses a problem unique to fs/ext4).

I'll pick this up. I've been away for a few weeks and I'm currently
on vacation, but I'll push this with a few other pending patches
for .35 on monday.
Theodore Ts'o - July 28, 2010, 11:15 p.m.
On Wed, Jul 28, 2010 at 08:12:15PM +0200, Jens Axboe wrote:
> 
> I'll pick this up. I've been away for a few weeks and I'm currently
> on vacation, but I'll push this with a few other pending patches
> for .35 on monday.

Great, thanks!!

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index baf5258..dbb510c 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -933,7 +933,7 @@  static inline int sb_issue_discard(struct super_block *sb,
 {
 	block <<= (sb->s_blocksize_bits - 9);
 	nr_blocks <<= (sb->s_blocksize_bits - 9);
-	return blkdev_issue_discard(sb->s_bdev, block, nr_blocks, GFP_KERNEL,
+	return blkdev_issue_discard(sb->s_bdev, block, nr_blocks, GFP_NOFS,
 				   BLKDEV_IFL_WAIT | BLKDEV_IFL_BARRIER);
 }