On Wed, 2022-08-03 at 09:13 +0200, Ilya Dryomov wrote:
> On Thu, Jun 30, 2022 at 10:21 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> >
> > This function always returns 0, and ignores the nofail boolean. Drop the
> > nofail argument, make the function void return and fix up the callers.
> >
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > drivers/block/rbd.c | 6 +++---
> > fs/ceph/addr.c | 32 ++++++++++++--------------------
> > fs/ceph/file.c | 32 +++++++++++++-------------------
> > include/linux/ceph/osd_client.h | 5 ++---
> > net/ceph/osd_client.c | 15 ++++++---------
> > 5 files changed, 36 insertions(+), 54 deletions(-)
> >
> > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> > index 91e541aa1f64..a8af0329ab77 100644
> > --- a/drivers/block/rbd.c
> > +++ b/drivers/block/rbd.c
> > @@ -1297,7 +1297,7 @@ static void rbd_osd_submit(struct ceph_osd_request *osd_req)
> > dout("%s osd_req %p for obj_req %p objno %llu %llu~%llu\n",
> > __func__, osd_req, obj_req, obj_req->ex.oe_objno,
> > obj_req->ex.oe_off, obj_req->ex.oe_len);
> > - ceph_osdc_start_request(osd_req->r_osdc, osd_req, false);
> > + ceph_osdc_start_request(osd_req->r_osdc, osd_req);
> > }
> >
> > /*
> > @@ -2081,7 +2081,7 @@ static int rbd_object_map_update(struct rbd_obj_request *obj_req, u64 snap_id,
> > if (ret)
> > return ret;
> >
> > - ceph_osdc_start_request(osdc, req, false);
> > + ceph_osdc_start_request(osdc, req);
> > return 0;
> > }
> >
> > @@ -4768,7 +4768,7 @@ static int rbd_obj_read_sync(struct rbd_device *rbd_dev,
> > if (ret)
> > goto out_req;
> >
> > - ceph_osdc_start_request(osdc, req, false);
> > + ceph_osdc_start_request(osdc, req);
> > ret = ceph_osdc_wait_request(osdc, req);
> > if (ret >= 0)
> > ceph_copy_from_page_vector(pages, buf, 0, ret);
> > diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> > index fe6147f20dee..66dc7844fcc6 100644
> > --- a/fs/ceph/addr.c
> > +++ b/fs/ceph/addr.c
> > @@ -357,9 +357,7 @@ static void ceph_netfs_issue_read(struct netfs_io_subrequest *subreq)
> > req->r_inode = inode;
> > ihold(inode);
> >
> > - err = ceph_osdc_start_request(req->r_osdc, req, false);
> > - if (err)
> > - iput(inode);
> > + ceph_osdc_start_request(req->r_osdc, req);
> > out:
> > ceph_osdc_put_request(req);
> > if (err)
>
> Hi Jeff,
>
> I'm confused by this err != 0 check. Previously err was set to 0
> by ceph_osdc_start_request() and netfs_subreq_terminated() was never
> called after an OSD request submission. Now it is called, but only if
> len != 0?
>
> I see that netfs_subreq_terminated() accepts either the amount of data
> transferred or an error code but it also has some transferred_or_error
> == 0 handling which this check effectively disables. And do we really
> want to account for transferred data before the transfer occurs?
>
> Thanks,
>
> Ilya
The incremental "patch to the patch" is here:
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 14793fabc26e..ec76e77f8d4b 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -337,6 +337,7 @@ static void ceph_netfs_issue_read(struct netfs_io_subrequest *subreq)
/* should always give us a page-aligned read */
WARN_ON_ONCE(page_off);
len = err;
+ err = 0;
osd_req_op_extent_osd_data_pages(req, 0, pages, len, 0, false, false);
req->r_callback = finish_netfs_read;
I'll need to rebuild my test environment to make sure this is working as expected.
--
Jeff Layton <jlayton@xxxxxxxxxx>
