From: Ohad Ben-Cohen <ohad@wizery.com> To: Andrew Morton <akpm@linux-foundation.org> Cc: linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Greg KH <greg@kroah.com>, Tony Lindgren <tony@atomide.com>, Benoit Cousson <b-cousson@ti.com>, Grant Likely <grant.likely@secretlab.ca>, Suman Anna <s-anna@ti.com>, Kevin Hilman <khilman@ti.com>, Arnd Bergmann <arnd@arndb.de>, Paul Walmsley <paul@pwsan.com>, Hari Kanigeri <hari.kanigeri@gmail.com>, Simon Que <simonque@gmail.com> Subject: Re: [PATCH v4 1/4] drivers: hwspinlock: add framework Date: Tue, 1 Feb 2011 08:20:13 +0200 [thread overview] Message-ID: <AANLkTik-fgCF_W0-DBE6UVTpjqF6Jf=u7bQYkcweEegQ@mail.gmail.com> (raw) In-Reply-To: <20110131153831.dca62146.akpm@linux-foundation.org> On Tue, Feb 1, 2011 at 1:38 AM, Andrew Morton <akpm@linux-foundation.org> wrote: > It's a little irritating having two hwspinlock.h's. > hwspinlock_internal.h wold be a conventional approach. But it's not a > big deal. ... >> +/** >> + * __hwspin_lock_timeout() - lock an hwspinlock with timeout limit >> + * @hwlock: the hwspinlock to be locked >> + * @timeout: timeout value in jiffies > > hm, why in jiffies? > > The problem here is that lazy programmers will use > > hwspin_lock_timeout(lock, 10, ...) > > and their code will work happily with HZ=100 but will explode with HZ=1000. > > IOW, this interface *requires* that all callers perform a > seconds-to-jiffies conversion before calling hwspin_lock_timeout(). So > why not reduce their effort and their ability to make mistakes by > defining the API to take seconds? I considered that, but then decided to use jiffies in order to be consistent with wait_event_timeout/schedule_timeout (although I don't return the remaining jiffies in case the lock is taken before the timeout elapses), and also to allow user-selected granularity. But I do kind of like the idea of not using jiffies. We can probably even move to msecs, since anyway this is an error condition, and people who needs a quick check should just use the trylock() version. I'll do a quick respin of the patches with that and the hwspinlock_internal.h comment above. Thanks, Ohad.
WARNING: multiple messages have this Message-ID (diff)
From: ohad@wizery.com (Ohad Ben-Cohen) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v4 1/4] drivers: hwspinlock: add framework Date: Tue, 1 Feb 2011 08:20:13 +0200 [thread overview] Message-ID: <AANLkTik-fgCF_W0-DBE6UVTpjqF6Jf=u7bQYkcweEegQ@mail.gmail.com> (raw) In-Reply-To: <20110131153831.dca62146.akpm@linux-foundation.org> On Tue, Feb 1, 2011 at 1:38 AM, Andrew Morton <akpm@linux-foundation.org> wrote: > It's a little irritating having two hwspinlock.h's. > hwspinlock_internal.h wold be a conventional approach. ?But it's not a > big deal. ... >> +/** >> + * __hwspin_lock_timeout() - lock an hwspinlock with timeout limit >> + * @hwlock: the hwspinlock to be locked >> + * @timeout: timeout value in jiffies > > hm, why in jiffies? > > The problem here is that lazy programmers will use > > ? ? ? ?hwspin_lock_timeout(lock, 10, ...) > > and their code will work happily with HZ=100 but will explode with HZ=1000. > > IOW, this interface *requires* that all callers perform a > seconds-to-jiffies conversion before calling hwspin_lock_timeout(). ?So > why not reduce their effort and their ability to make mistakes by > defining the API to take seconds? I considered that, but then decided to use jiffies in order to be consistent with wait_event_timeout/schedule_timeout (although I don't return the remaining jiffies in case the lock is taken before the timeout elapses), and also to allow user-selected granularity. But I do kind of like the idea of not using jiffies. We can probably even move to msecs, since anyway this is an error condition, and people who needs a quick check should just use the trylock() version. I'll do a quick respin of the patches with that and the hwspinlock_internal.h comment above. Thanks, Ohad.
next prev parent reply other threads:[~2011-02-01 6:21 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-01-31 10:33 [PATCH v4 0/4] Introduce hardware spinlock framework Ohad Ben-Cohen 2011-01-31 10:33 ` Ohad Ben-Cohen 2011-01-31 10:33 ` Ohad Ben-Cohen 2011-01-31 10:33 ` [PATCH v4 1/4] drivers: hwspinlock: add framework Ohad Ben-Cohen 2011-01-31 10:33 ` Ohad Ben-Cohen 2011-01-31 10:33 ` Ohad Ben-Cohen 2011-01-31 23:38 ` Andrew Morton 2011-01-31 23:38 ` Andrew Morton 2011-01-31 23:38 ` Andrew Morton 2011-02-01 6:20 ` Ohad Ben-Cohen [this message] 2011-02-01 6:20 ` Ohad Ben-Cohen 2011-02-01 6:38 ` Andrew Morton 2011-02-01 6:38 ` Andrew Morton 2011-02-01 7:36 ` Ohad Ben-Cohen 2011-02-01 7:36 ` Ohad Ben-Cohen 2011-02-01 7:40 ` Andrew Morton 2011-02-01 7:40 ` Andrew Morton 2011-02-01 8:12 ` Ohad Ben-Cohen 2011-02-01 8:12 ` Ohad Ben-Cohen 2011-02-02 12:11 ` Russell King - ARM Linux 2011-02-02 12:11 ` Russell King - ARM Linux 2011-02-02 12:11 ` Russell King - ARM Linux 2011-02-04 1:47 ` Tony Lindgren 2011-02-04 1:47 ` Tony Lindgren 2011-02-01 14:17 ` Greg KH 2011-02-01 14:17 ` Greg KH 2011-02-01 15:34 ` Arnd Bergmann 2011-02-01 15:34 ` Arnd Bergmann 2011-01-31 10:33 ` [PATCH v4 2/4] drivers: hwspinlock: add OMAP implementation Ohad Ben-Cohen 2011-01-31 10:33 ` Ohad Ben-Cohen 2011-01-31 10:33 ` Ohad Ben-Cohen 2011-01-31 10:33 ` [PATCH v4 3/4] OMAP4: hwmod data: Add hwspinlock Ohad Ben-Cohen 2011-01-31 10:33 ` Ohad Ben-Cohen 2011-01-31 10:33 ` Ohad Ben-Cohen 2011-01-31 10:33 ` [PATCH v4 4/4] omap: add hwspinlock device Ohad Ben-Cohen 2011-01-31 10:33 ` Ohad Ben-Cohen 2011-01-31 10:33 ` Ohad Ben-Cohen
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='AANLkTik-fgCF_W0-DBE6UVTpjqF6Jf=u7bQYkcweEegQ@mail.gmail.com' \ --to=ohad@wizery.com \ --cc=akpm@linux-foundation.org \ --cc=arnd@arndb.de \ --cc=b-cousson@ti.com \ --cc=grant.likely@secretlab.ca \ --cc=greg@kroah.com \ --cc=hari.kanigeri@gmail.com \ --cc=khilman@ti.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-omap@vger.kernel.org \ --cc=paul@pwsan.com \ --cc=s-anna@ti.com \ --cc=simonque@gmail.com \ --cc=tony@atomide.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: linkBe 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.