[Cluster-devel] [PATCH RESEND] gfs2: Return more useful errors from gfs2_rgrp_send_discards()

[Andrew Price <anprice@redhat.com>]

On 19/04/2022 16:49, Andrew Price wrote:
> On 05/04/2022 13:08, Andrew Price wrote:
>> The bug that 27ca8273f ("gfs2: Make sure FITRIM minlen is rounded up to
>> fs block size") fixes was a little confusing as the user saw
>> "Input/output error" which masked the -EINVAL that sb_issue_discard()
>> returned.
>>
>> sb_issue_discard() can fail for various reasons, so we should return its
>> return value from gfs2_rgrp_send_discards() to avoid all errors being
>> reported as IO errors.
>>
>> This improves error reporting for FITRIM and makes no difference to the
>> -o discard code path because the return value from
>> gfs2_rgrp_send_discards() gets thrown away in that case (and the option
>> switches off). Presumably that's why it was ok to just return -EIO in
>> the past, before FITRIM was implemented.
>>
>> Tested with xfstests.
> 
> Can I get a thumbs-up or thumbs-down for this patch? It's pretty 
> straightforward. I just don't want it to get forgotten about.
> 
> Cheers,
> Andy

Please could you take this patch?

Cheers,
Andy

>> Signed-off-by: Andrew Price <anprice@redhat.com>
>> ---
>>
>> I don't see this in for-next yet so I've updated the commit log to 
>> include more
>> details.
>>
>> ? fs/gfs2/rgrp.c | 4 ++--
>> ? 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
>> index 801ad9f4f2be..886343cc05ab 100644
>> --- a/fs/gfs2/rgrp.c
>> +++ b/fs/gfs2/rgrp.c
>> @@ -1315,7 +1315,7 @@ int gfs2_rgrp_send_discards(struct gfs2_sbd 
>> *sdp, u64 offset,
>> ????? u64 blk;
>> ????? sector_t start = 0;
>> ????? sector_t nr_blks = 0;
>> -??? int rv;
>> +??? int rv = -EIO;
>> ????? unsigned int x;
>> ????? u32 trimmed = 0;
>> ????? u8 diff;
>> @@ -1371,7 +1371,7 @@ int gfs2_rgrp_send_discards(struct gfs2_sbd 
>> *sdp, u64 offset,
>> ????? if (sdp->sd_args.ar_discard)
>> ????????? fs_warn(sdp, "error %d on discard request, turning discards 
>> off for this filesystem\n", rv);
>> ????? sdp->sd_args.ar_discard = 0;
>> -??? return -EIO;
>> +??? return rv;
>> ? }
>> ? /**
>