Messages in this thread Patch in this message | | | Date | Tue, 25 Oct 2022 19:17:10 +0800 | Subject | Re: [PATCH v2] btrfs: volumes: Increase bioc pointer check | From | Qu Wenruo <> |
| |
On 2022/10/25 18:52, Li zeming wrote: > This patch has the following changes: > 1. Modify "is returned" in the comments to "should be returned". > 2. Remove the __GFP_NOFAIL flag from the kzalloc function, which returns > NULL if kzalloc fails to allocate memory for bioc.
Firstly this part should be in change log, not commit message.
You can just do a search in the mail list and see how we handle patches with newer versions.
Secondly, you didn't mention why we can remove the __GFP_NOFAIL flag at all.
The commit message should look like this instead:
``` Currently we allocate memory for btrfs_io_context using (GFP_NOFS|__GFP_NOFAIL) in alloc_btrfs_io_context().
But there is nothing special for that function to require NOFAIL flag.
Furthermore the only caller of alloc_btrfs_io_context() is already handling the ENOMEM error properly.
Thus we can safely remove the __GFP_NOFAIL flag, and handle allocation failure properly. ``` > > Reviewed-by: Qu Wenruo <wqu@suse.com>
I'd say, please don't add my tag until everything is fine. I did a wrong expectation.
Thanks, Qu
> Signed-off-by: Li zeming <zeming@nfschina.com> > --- > v2: Add annotation vocabulary modify, remove __GFP_NOFAIL flag. > > fs/btrfs/volumes.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 064ab2a79c80..b8d901f58995 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -5891,7 +5891,9 @@ static struct btrfs_io_context *alloc_btrfs_io_context(struct btrfs_fs_info *fs_ > * and the stripes. > */ > sizeof(u64) * (total_stripes), > - GFP_NOFS|__GFP_NOFAIL); > + GFP_NOFS); > + if (!bioc) > + return NULL; > > atomic_set(&bioc->error, 0); > refcount_set(&bioc->refs, 1); > @@ -6071,7 +6073,7 @@ struct btrfs_discard_stripe *btrfs_map_discard(struct btrfs_fs_info *fs_info, > * array of stripes. > * For READ, it also needs to be supported using the same mirror number. > * > - * If the requested block is not left of the left cursor, EIO is returned. This > + * If the requested block is not left of the left cursor, EIO should be returned. This > * can happen because btrfs_num_copies() returns one more in the dev-replace > * case. > */
| |