From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Price <anprice@redhat.com>
Date: Mon, 9 May 2022 11:41:14 +0100
Subject: [Cluster-devel] [PATCH RESEND] gfs2: Return more useful errors
 from gfs2_rgrp_send_discards()
In-Reply-To: <4f808327-b831-f546-8d3f-5dfdf849bdef@redhat.com>
References: <20220405120847.583327-1-anprice@redhat.com>
 <b5fcfcbd-104f-ea1a-f808-769c7bd8b91f@redhat.com>
 <4f808327-b831-f546-8d3f-5dfdf849bdef@redhat.com>
Message-ID: <5488b93e-96c8-9007-9ee4-667247fa8c30@redhat.com>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

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