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?
>
No we don't. I think you're correct. What I'm not sure of is why this
doesn't cause test failures all over the place.
In any case, I'll need to respin this. I'll do that and send a v2.
Good catch!
--
Jeff Layton <jlayton@xxxxxxxxxx>
