lkml.org 
[lkml]   [2017]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] staging: lustre: avoid going through unlock/lock overhead
On Thu, Sep 07, 2017 at 02:33:49PM +0200, Greg KH wrote:
> On Thu, Sep 07, 2017 at 01:57:42PM +0300, Cihangir Akturk wrote:
> > Unlocking a spin lock and then immediately locking without doing
> > anything useful in between buys us nothing, except wasting CPU cycles.
>
> Not always, it can be a "gate" for other users of the lock.

OK, I get it.


> Are you sure that is not what is going on here?

No, I'm not sure. But yes, that's possible it might be used to let in
other users of the lock.

> Did you test this out on a lustre system? The locks here are
> anything but trivial...

Unfortunately I haven't tested this change on a lustre system. Just
compile-tested.

> >
> > Also code size gets smaller.
> >
> > Before:
> >
> > text data bss dec hex filename
> > 70415 2356 4108 76879 12c4f drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.o
> >
> > After:
> >
> > text data bss dec hex filename
> > 70095 2356 4108 76559 12b0f drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.o
> >
> > Signed-off-by: Cihangir Akturk <cakturk@gmail.com>
> > ---
> > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c | 18 ++++++++----------
> > 1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> > index 64763aa..5d9cd33 100644
> > --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> > +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> > @@ -1624,8 +1624,9 @@ int kiblnd_fmr_pool_map(struct kib_fmr_poolset *fps, struct kib_tx *tx,
> > __u64 version;
> > int rc;
> >
> > - again:
> > +again:
> > spin_lock(&fps->fps_lock);
> > +again_locked:
> > version = fps->fps_version;
> > list_for_each_entry(fpo, &fps->fps_pool_list, fpo_list) {
> > fpo->fpo_deadline = cfs_time_shift(IBLND_POOL_DEADLINE);
> > @@ -1722,10 +1723,8 @@ int kiblnd_fmr_pool_map(struct kib_fmr_poolset *fps, struct kib_tx *tx,
> > }
> >
> > /* EAGAIN and ... */
> > - if (version != fps->fps_version) {
> > - spin_unlock(&fps->fps_lock);
> > - goto again;
> > - }
> > + if (version != fps->fps_version)
> > + goto again_locked;
> > }
> >
> > if (fps->fps_increasing) {
> > @@ -1754,9 +1753,8 @@ int kiblnd_fmr_pool_map(struct kib_fmr_poolset *fps, struct kib_tx *tx,
> > } else {
> > fps->fps_next_retry = cfs_time_shift(IBLND_POOL_RETRY);
> > }
> > - spin_unlock(&fps->fps_lock);
> >
> > - goto again;
> > + goto again_locked;
>
> Really, gotos backwards? Ick, that's horrid as well, so maybe this is
> better? I hate this whole codebase...
>
> I'll let the Lustre maintainers decide about this one...
>
> greg k-h

\
 
 \ /
  Last update: 2017-09-07 15:30    [W:0.037 / U:11.636 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site