On Tue, Nov 26, 2019 at 7:25 PM Xiubo Li <xiubli@xxxxxxxxxx> wrote:
>
> On 2019/11/26 19:03, Yan, Zheng wrote:
> > On 11/26/19 6:01 PM, Xiubo Li wrote:
> >> On 2019/11/26 17:49, Yan, Zheng wrote:
> >>> On Tue, Nov 26, 2019 at 4:57 PM <xiubli@xxxxxxxxxx> wrote:
> >>>> From: Xiubo Li <xiubli@xxxxxxxxxx>
> >>>>
> >>>> The nr in ceph_reclaim_caps_nr() is very possibly larger than 1,
> >>>> so we may miss it and the reclaim work couldn't triggered as expected.
> >>>>
> >>>> Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
> >>>> ---
> >>>> fs/ceph/mds_client.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> >>>> index 08b70b5ee05e..547ffe16f91c 100644
> >>>> --- a/fs/ceph/mds_client.c
> >>>> +++ b/fs/ceph/mds_client.c
> >>>> @@ -2020,7 +2020,7 @@ void ceph_reclaim_caps_nr(struct
> >>>> ceph_mds_client *mdsc, int nr)
> >>>> if (!nr)
> >>>> return;
> >>>> val = atomic_add_return(nr, &mdsc->cap_reclaim_pending);
> >>>> - if (!(val % CEPH_CAPS_PER_RELEASE)) {
> >>>> + if (val / CEPH_CAPS_PER_RELEASE) {
> >>>> atomic_set(&mdsc->cap_reclaim_pending, 0);
> >>>> ceph_queue_cap_reclaim_work(mdsc);
> >>>> }
> >>> this will call ceph_queue_cap_reclaim_work too frequently
> >>
> >> No it won't, the '/' here equals to '>=' and then the
> >> "mdsc->cap_reclaim_pending" will be reset and it will increase from 0
> >> again.
> >>
> >> It will make sure that only when "mdsc->cap_reclaim_pending >=
> >> CEPH_CAPS_PER_RELEASE" will call the work queue.
> >
> > Work does not get executed immediately. call
> > ceph_queue_cap_reclaim_work() when val == CEPH_CAPS_PER_RELEASE is
> > enough. There is no point to call it too frequently
> >
> >
> Yeah, it true and I am okay with this. Just going through the session
> release related code, and saw the "nr" parameter will be "ctx->used" in
> ceph_reclaim_caps_nr(mdsc, ctx->used), and in case there has many
> sessions with tremendous amount of caps. In corner case that we may
> always miss the condition that the "val == CEPH_CAPS_PER_RELEASE" here.
>
good catch. But the test should be something like
"if ((val % CEPH_CAPS_PER_RELEASE) < nr)"
> IMO, it wants to fire the work queue once "val >=
> CEPH_CAPS_PER_RELEASE", but it is not working like this, the val may
> just skip it without doing any thing.
>
> Thanks
>
>
> >>
> >>>> --
> >>>> 2.21.0
> >>>>
> >>
> >
>
