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

[Andrew Price <anprice@redhat.com>]

On 29/04/2022 15:08, Andrew Price wrote:
> 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

For convenience, I've pushed this patch to gitlab:

   git fetch https://gitlab.com/andyprice/linux.git discard_errors
   git show FETCH_HEAD

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;
>>> ? }
>>> ? /**
>>
>