From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Price Date: Fri, 29 Apr 2022 15:08:36 +0100 Subject: [Cluster-devel] [PATCH RESEND] gfs2: Return more useful errors from gfs2_rgrp_send_discards() In-Reply-To: References: <20220405120847.583327-1-anprice@redhat.com> Message-ID: <4f808327-b831-f546-8d3f-5dfdf849bdef@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 >> --- >> >> 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; >> ? } >> ? /** >