Patchwork mtd: add documentation about locking context of MTD API

login
register
mail settings
Submitter Robert Jarzmik
Date March 19, 2012, 10:26 p.m.
Message ID <1332195985-21043-1-git-send-email-robert.jarzmik@free.fr>
Download mbox | patch
Permalink /patch/147664/
State New
Headers show

Comments

Robert Jarzmik - March 19, 2012, 10:26 p.m.
Add a comment to mtd header for MTD drivers writters, so that they
know that each function in the MTD API, ie. in the mtd_info
structure, is called in a sleeping context.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 include/linux/mtd/mtd.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Richard Weinberger - March 19, 2012, 10:31 p.m.
Am 19.03.2012 23:26, schrieb Robert Jarzmik:
> Add a comment to mtd header for MTD drivers writters, so that they
> know that each function in the MTD API, ie. in the mtd_info
> structure, is called in a sleeping context.
> 

Why do we need this comment?
"sleeping" context isn't special, it's the default.
atomic would be...

Thanks,
//richard
Robert Jarzmik - March 19, 2012, 10:51 p.m.
Richard Weinberger <richard@nod.at> writes:

> Am 19.03.2012 23:26, schrieb Robert Jarzmik:
>> Add a comment to mtd header for MTD drivers writters, so that they
>> know that each function in the MTD API, ie. in the mtd_info
>> structure, is called in a sleeping context.
>> 
>
> Why do we need this comment?
Because I was asked to, in [1].

> "sleeping" context isn't special, it's the default.
> atomic would be...
Ah, well, maybe. From what I've seen in the usb drivers area, portions of code
are called from interrupts, through hooks. As a driver writer, I'm always
interested to know whether I can msleep() or not.

Cheers.

--
Robert

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-March/040236.html
Artem Bityutskiy - March 20, 2012, 11:44 a.m.
On Mon, 2012-03-19 at 23:51 +0100, Robert Jarzmik wrote:
> Richard Weinberger <richard@nod.at> writes:
> 
> > Am 19.03.2012 23:26, schrieb Robert Jarzmik:
> >> Add a comment to mtd header for MTD drivers writters, so that they
> >> know that each function in the MTD API, ie. in the mtd_info
> >> structure, is called in a sleeping context.
> >> 
> >
> > Why do we need this comment?
> Because I was asked to, in [1].

Well, Richard has a point :-) But I guess you can simply write "may
sleep" instead of "sleeping context". Or may be even use "may_sleep()"
in the wrappers?
Robert Jarzmik - March 21, 2012, 8:20 p.m.
Artem Bityutskiy <dedekind1@gmail.com> writes:

> On Mon, 2012-03-19 at 23:51 +0100, Robert Jarzmik wrote:
>> Richard Weinberger <richard@nod.at> writes:
>> 
>> > Am 19.03.2012 23:26, schrieb Robert Jarzmik:
>> >> Add a comment to mtd header for MTD drivers writters, so that they
>> >> know that each function in the MTD API, ie. in the mtd_info
>> >> structure, is called in a sleeping context.
>> >> 
>> >
>> > Why do we need this comment?
>> Because I was asked to, in [1].
>
> Well, Richard has a point :-)
Ok ... but I don't get it. I think it's because my previous statement "As a
driver writer, I'm always interested to know whether I can msleep() or not." is
not good enough ... Neither is my USB example (I was thinking of commit id
5e23e90f33888769ffe253663cc5f3ea0bb6da49 btw).

> But I guess you can simply write "may sleep" instead of "sleeping context". Or
> may be even use "may_sleep()" in the wrappers?
may_sleep() doesn't exist AFAIK. might_sleep() does exist, but it's already
mentioned in my patch, and because you're talking about may_sleep() while I used
might_sleep(), I'm confused.

Do you want me to take my patch and do a simple 's/in a sleeping context/and may
sleep/g', or is it more complicated ?
Artem Bityutskiy - March 22, 2012, 9:37 a.m.
On Wed, 2012-03-21 at 21:20 +0100, Robert Jarzmik wrote:
> Do you want me to take my patch and do a simple 's/in a sleeping context/and may
> sleep/g', or is it more complicated ?

Yes, I guess you can put just a short comment like "these all may
sleep".

WRT to 'might_sleep()' - you can put these to the _code_, to the mtd
wrappers like 'mtd_write()', I suppose.

Patch

diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index cf5ea8c..4a417e7 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -176,6 +176,11 @@  struct mtd_info {
 	/*
 	 * Do not call via these pointers, use corresponding mtd_*()
 	 * wrappers instead.
+	 *
+	 * All the following functions are called in a sleeping context, ie. it
+	 * can be safely assumed in mtd drivers that :
+	 *  - might_sleep() == 1
+	 *  - msleep(), mutex_lock() and co are authorized
 	 */
 	int (*_erase) (struct mtd_info *mtd, struct erase_info *instr);
 	int (*_point) (struct mtd_info *mtd, loff_t from, size_t len,