All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sahitya Tummala <stummala@codeaurora.org>
To: David Laight <David.Laight@ACULAB.COM>
Cc: 'Chao Yu' <yuchao0@huawei.com>, Jaegeuk Kim <jaegeuk@kernel.org>,
	"linux-f2fs-devel@lists.sourceforge.net" 
	<linux-f2fs-devel@lists.sourceforge.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] f2fs: change to use rwsem for cp_mutex
Date: Wed, 25 Nov 2020 18:38:51 +0530	[thread overview]
Message-ID: <20201125130851.GA22157@codeaurora.org> (raw)
In-Reply-To: <effbe4f5edaf4d45a64d12c65e0dc6b0@AcuMS.aculab.com>

Hi David,

On Tue, Nov 24, 2020 at 09:12:12AM +0000, David Laight wrote:
> From: Chao Yu
> > Sent: 24 November 2020 03:12
> > 
> > On 2020/11/24 1:05, David Laight wrote:
> > > From: Sahitya Tummala
> > >> Sent: 23 November 2020 05:29
> > >>
> > >> Use rwsem to ensure serialization of the callers and to avoid
> > >> starvation of high priority tasks, when the system is under
> > >> heavy IO workload.
> > >
> > > I can't see any read lock requests.
> > >
> > > So why the change?
> > 
> > Hi David,
> > 
> > You can check the context of this patch in below link:
> > 
> > https://lore.kernel.org/linux-f2fs-devel/8e094021b958f9fe01df1183a2677882@codeaurora.org/T/#t
> > 
> > BTW, the root cause here is that mutex lock won't serialize callers, so there
> > could be potential starvation problem when this lock is always grabbed by high
> > priority tasks.
> 
> That doesn't seem right.
> 
> If I read the above correctly it was high priority tasks that were
> being 'starved' precisely because mutex lock serializes wakers.

Actually it can happen for any random task irrespective of the priority.
In my case, I was observing that the thread that went to sleep first is
not able to acquire the lock first and other new threads that came in
just around the mutex unlock time were acquiring the lock.

> 
> If you have a lock that is contended so much that it is held 100%
> of the time you need a different locking strategy.
> 
> IIRC mutex locks are 'ticket' locks so that only one thread is woken
> each time the mutex is released, and they are woken in the order
> they went to sleep.

AFAIK mutex locks doesn't *strictly* enforce FIFO order. The lock is released
before waking the first waiting task. The waiting task has to run to claim
the lock. So the lock is available for other tasks in this *short* window.

Thanks,

> 
> While this behaviour might not be the one you want, relying on
> rwsem (which might happen currently to work differently) doesn't
> seem the correct long term fix.
> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)

-- 
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

WARNING: multiple messages have this Message-ID (diff)
From: Sahitya Tummala <stummala@codeaurora.org>
To: David Laight <David.Laight@ACULAB.COM>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-f2fs-devel@lists.sourceforge.net"
	<linux-f2fs-devel@lists.sourceforge.net>
Subject: Re: [f2fs-dev] [PATCH] f2fs: change to use rwsem for cp_mutex
Date: Wed, 25 Nov 2020 18:38:51 +0530	[thread overview]
Message-ID: <20201125130851.GA22157@codeaurora.org> (raw)
In-Reply-To: <effbe4f5edaf4d45a64d12c65e0dc6b0@AcuMS.aculab.com>

Hi David,

On Tue, Nov 24, 2020 at 09:12:12AM +0000, David Laight wrote:
> From: Chao Yu
> > Sent: 24 November 2020 03:12
> > 
> > On 2020/11/24 1:05, David Laight wrote:
> > > From: Sahitya Tummala
> > >> Sent: 23 November 2020 05:29
> > >>
> > >> Use rwsem to ensure serialization of the callers and to avoid
> > >> starvation of high priority tasks, when the system is under
> > >> heavy IO workload.
> > >
> > > I can't see any read lock requests.
> > >
> > > So why the change?
> > 
> > Hi David,
> > 
> > You can check the context of this patch in below link:
> > 
> > https://lore.kernel.org/linux-f2fs-devel/8e094021b958f9fe01df1183a2677882@codeaurora.org/T/#t
> > 
> > BTW, the root cause here is that mutex lock won't serialize callers, so there
> > could be potential starvation problem when this lock is always grabbed by high
> > priority tasks.
> 
> That doesn't seem right.
> 
> If I read the above correctly it was high priority tasks that were
> being 'starved' precisely because mutex lock serializes wakers.

Actually it can happen for any random task irrespective of the priority.
In my case, I was observing that the thread that went to sleep first is
not able to acquire the lock first and other new threads that came in
just around the mutex unlock time were acquiring the lock.

> 
> If you have a lock that is contended so much that it is held 100%
> of the time you need a different locking strategy.
> 
> IIRC mutex locks are 'ticket' locks so that only one thread is woken
> each time the mutex is released, and they are woken in the order
> they went to sleep.

AFAIK mutex locks doesn't *strictly* enforce FIFO order. The lock is released
before waking the first waiting task. The waiting task has to run to claim
the lock. So the lock is available for other tasks in this *short* window.

Thanks,

> 
> While this behaviour might not be the one you want, relying on
> rwsem (which might happen currently to work differently) doesn't
> seem the correct long term fix.
> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)

-- 
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2020-11-25 13:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-23  5:28 [PATCH] f2fs: change to use rwsem for cp_mutex Sahitya Tummala
2020-11-23  5:28 ` [f2fs-dev] " Sahitya Tummala
2020-11-23 17:05 ` David Laight
2020-11-23 17:05   ` [f2fs-dev] " David Laight
2020-11-24  3:12   ` Chao Yu
2020-11-24  3:12     ` [f2fs-dev] " Chao Yu
2020-11-24  9:12     ` David Laight
2020-11-24  9:12       ` [f2fs-dev] " David Laight
2020-11-25 13:08       ` Sahitya Tummala [this message]
2020-11-25 13:08         ` Sahitya Tummala
2020-11-24  3:06 ` Chao Yu
2020-11-24  3:06   ` [f2fs-dev] " Chao Yu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201125130851.GA22157@codeaurora.org \
    --to=stummala@codeaurora.org \
    --cc=David.Laight@ACULAB.COM \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yuchao0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.