* Re: [PATCH 0/8] Use correctly the Xen memory terminologies in Linux
From: Julien Grall @ 2015-07-29 11:02 UTC (permalink / raw)
To: xen-devel
Cc: ian.campbell, stefano.stabellini, linux-kernel, Boris Ostrovsky,
David Vrabel, Dmitry Torokhov, Greg Kroah-Hartman, H. Peter Anvin,
Ingo Molnar, James E.J. Bottomley,
Jean-Christophe Plagniol-Villard, Jiri Slaby, Juergen Gross,
Konrad Rzeszutek Wilk, linux-api, linux-arm-kernel, linux-fbdev,
linux-input, linuxppc-dev, linux-scsi, netdev,
Roger Pau Monné, Russell King
In-Reply-To: <1438095769-2560-1-git-send-email-julien.grall@citrix.com>
On 28/07/15 16:02, Julien Grall wrote:
> Hi all,
>
> This patch series aims to use the memory terminologies described in
> include/linux/mm.h [1] for Linux xen code.
I mistakenly wrote the wrong include here. It should be include/xen/mm.h
from the Xen tree:
http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e758ed14f390342513405dd766e874934573e6cb
Regards,
--
Julien Grall
^ permalink raw reply
* Re: [PATCH 0/4] enable migration of driver pages
From: Daniel Vetter @ 2015-07-29 10:55 UTC (permalink / raw)
To: Mel Gorman
Cc: aquini, mst, linux-api, linux-kernel, dri-devel, virtualization,
bfields, minchan, Gioh Kim, linux-mm, viro, akpm, Gioh Kim,
linux-fsdevel, jlayton, koct9i, iamjoonsoo.kim, vbabka
In-Reply-To: <20150729104945.GA30872@techsingularity.net>
On Wed, Jul 29, 2015 at 11:49:45AM +0100, Mel Gorman wrote:
> On Mon, Jul 13, 2015 at 05:35:15PM +0900, Gioh Kim wrote:
> > My ARM-based platform occured severe fragmentation problem after long-term
> > (several days) test. Sometimes even order-3 page allocation failed. It has
> > memory size 512MB ~ 1024MB. 30% ~ 40% memory is consumed for graphic processing
> > and 20~30 memory is reserved for zram.
> >
>
> The primary motivation of this series is to reduce fragmentation by allowing
> more kernel pages to be moved. Conceptually that is a worthwhile goal but
> there should be at least one major in-kernel user and while balloon
> pages were a good starting point, I think we really need to see what the
> zram changes look like at the same time.
I think gpu drivers really would be the perfect candidate for compacting
kernel page allocations. And this also seems the primary motivation for
this patch series, so I think that's really what we should use to judge
these patches.
Of course then there's the seemingly eternal chicken/egg problem of
upstream gpu drivers for SoCs :(
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 2/4] mm/compaction: enable mobile-page migration
From: Mel Gorman @ 2015-07-29 10:52 UTC (permalink / raw)
To: Gioh Kim
Cc: jlayton, bfields, vbabka, iamjoonsoo.kim, viro, mst, koct9i,
minchan, aquini, linux-fsdevel, virtualization, linux-kernel,
linux-api, linux-mm, dri-devel, akpm, Gioh Kim
In-Reply-To: <1436776519-17337-3-git-send-email-gioh.kim@lge.com>
On Mon, Jul 13, 2015 at 05:35:17PM +0900, Gioh Kim wrote:
> From: Gioh Kim <gurugio@hanmail.net>
>
> Add framework to register callback functions and check page mobility.
> There are some modes for page isolation so that isolate interface
> has arguments of page address and isolation mode while putback
> interface has only page address as argument.
>
> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> Acked-by: Rafael Aquini <aquini@redhat.com>
> ---
> fs/proc/page.c | 3 ++
> include/linux/compaction.h | 80 ++++++++++++++++++++++++++++++++++
> include/linux/fs.h | 2 +
> include/linux/page-flags.h | 19 ++++++++
> include/uapi/linux/kernel-page-flags.h | 1 +
> 5 files changed, 105 insertions(+)
>
An update to the address_space operations in
Documentation/filesystems/Locking and Documentation/filesystems/vfs.txt
is required. I was going to say "recommended" but it really is required.
The responsibilities and locking rules of these interfaces must be extremely
clear as you may be asking multiple driver authors to use this interface.
For example, it must be clear to users of these interfaces that the isolate
must prevent any parallel updates to the data, prevent parallel frees and
halt attempted accesses until migration is complete. It will not always
be obvious how to do this and may not be obvious that it is required if
someone has not experienced the joy that is mm/migrate.c. For example,
mapped LRU pages get unmapped with migration entries so faults that access
the data wait until the migration completes. Balloons, zram, graphics will
need to provide similar guarantees.
As data accesses may now sleep due to migration, drivers will need to
be careful that it is safe to sleep and suggest that they do not attempt
to spin.
Depending on how it is implemented, the putback may be responsible for
waking up any tasks waiting to access the page.
There are going to be more hazards here which is why documentation to spell
it out is ideal and that zram gets converted to find all the locking and
access pitfalls.
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 7eee2d8..a4f5a00 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -146,6 +146,9 @@ u64 stable_page_flags(struct page *page)
> if (PageBalloon(page))
> u |= 1 << KPF_BALLOON;
>
> + if (PageMobile(page))
> + u |= 1 << KPF_MOBILE;
> +
> u |= kpf_copy_bit(k, KPF_LOCKED, PG_locked);
>
> u |= kpf_copy_bit(k, KPF_SLAB, PG_slab);
> diff --git a/include/linux/compaction.h b/include/linux/compaction.h
> index aa8f61c..f693072 100644
> --- a/include/linux/compaction.h
> +++ b/include/linux/compaction.h
> @@ -1,6 +1,9 @@
> #ifndef _LINUX_COMPACTION_H
> #define _LINUX_COMPACTION_H
>
> +#include <linux/page-flags.h>
> +#include <linux/pagemap.h>
> +
> /* Return values for compact_zone() and try_to_compact_pages() */
> /* compaction didn't start as it was deferred due to past failures */
> #define COMPACT_DEFERRED 0
> @@ -51,6 +54,70 @@ extern void compaction_defer_reset(struct zone *zone, int order,
> bool alloc_success);
> extern bool compaction_restarting(struct zone *zone, int order);
>
> +static inline bool mobile_page(struct page *page)
> +{
> + return page->mapping && (PageMobile(page) || PageBalloon(page));
> +}
> +
This creates an oddity because now there is a disconnect between movable
and mobile pages. They are similar but different.
o A Mobile page is a driver-owned page that has the address space
operations that enable migration.
o A Movable page is generally a page mapped by page tables that can be
migrated using the existing mechanisms.
The concepts should be unified.
A Mobile page is a driver-owner page that has the address space
operations that enable migration. Pages that are mapped by userspace are
considered to be mobile with the following properties
a_ops->isolatepage isolates the page from the LRU to prevent
parallel reclaim. It is unmapped from page tables using rmap
with PTEs replaced by migration entries. Any attempt to access
the page will wait in page fault until the migration completes.
a_ops->putbackpage removes the migration entries and wakes up
all waiters in page fault.
A further property is that allocation of this type specified
__GFP_MOVABLE to group them all together. They are the most mobile
page category that are cheapest to move. In theory, all mobile
pages could be allocated __GFP_MOVABLE if it's known in advance
the page->mapping will have the necessary operations in the
future.
?
A complicating factor is that a Movable page as it's currently defined
may not have a page->mapping. You'd have to continue replying on PageLRU to
identify them as a special page that has access to the necessary isolateppage
and putbackpage helpers. However, at least we would have a single view
on what a movable page is.
Additional note: After I wrote the above, I read the other reviews. I
did not read them in advance so I'd have a fresh view. I see
Konstantin Khlebnikov has been active and he suggested the mobility
naming to distinguish between the LRU pages. I simply disagree
even though I see his reasoning. I do not think we should have a
special case of LRU pages and everything else. Instead we should
have a single concept of movability (or mobility) with the special
case being that LRU pages without an aops can directly call the
necessary helpers.
> +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t mode)
> +{
> + bool ret = false;
> +
> + /*
> + * Avoid burning cycles with pages that are yet under __free_pages(),
> + * or just got freed under us.
> + *
> + * In case we 'win' a race for a mobile page being freed under us and
> + * raise its refcount preventing __free_pages() from doing its job
> + * the put_page() at the end of this block will take care of
> + * release this page, thus avoiding a nasty leakage.
> + */
> + if (unlikely(!get_page_unless_zero(page)))
> + goto out;
> +
Ok.
> + /*
> + * As mobile pages are not isolated from LRU lists, concurrent
> + * compaction threads can race against page migration functions
> + * as well as race against the releasing a page.
> + *
> + * In order to avoid having an already isolated mobile page
> + * being (wrongly) re-isolated while it is under migration,
> + * or to avoid attempting to isolate pages being released,
> + * lets be sure we have the page lock
> + * before proceeding with the mobile page isolation steps.
> + */
> + if (unlikely(!trylock_page(page)))
> + goto out_putpage;
> +
There are some big assumptions here. It assumes that any users of this
interface can prevent parallel compaction attempts via the page lock. It
also assumes that the caller does not recursively hold the page lock already.
It would be incompatible with how LRU pages are isolated as they co-ordinate
via the zone->lru_lock.
I suspect you went with the page lock because it happens to be what the
balloon driver needed which is fine, but potentially pastes us into a
corner later.
I don't see a way this could be generically handled for arbitrary subsystems
unless you put responsibility for the locking inside a_ops->isolatepage. That
still works for existing movable pages if you give it a pseudo a_ops for
pages without page->mapping.
Because of this, I really think it would benefit if there was a patch
3 that converted the existing migration of LRU pages to use the aops
interface. This could be done via a fake address_space that only populates
the migration interfaces and is used for LRU pages. Then remove the LRU
special casing in compaction and migration before converting the balloon
driver and zram. This will rattle out any conceivable locking hazard and
unify migration in general. I recognise that it's a lot of heavy lifting
unfortunately but it leaves you with a partial solution to your problem
(zram in the way) and paves the way for drivers to reliably convert.
> + if (!(mobile_page(page) && page->mapping->a_ops->isolatepage))
> + goto out_not_isolated;
> + ret = page->mapping->a_ops->isolatepage(page, mode);
> + if (!ret)
> + goto out_not_isolated;
> + unlock_page(page);
> + return ret;
> +
> +out_not_isolated:
> + unlock_page(page);
> +out_putpage:
> + put_page(page);
> +out:
> + return ret;
> +}
> +
> +static inline void putback_mobilepage(struct page *page)
> +{
> + /*
> + * 'lock_page()' stabilizes the page and prevents races against
> + * concurrent isolation threads attempting to re-isolate it.
> + */
> + lock_page(page);
> + if (page->mapping && page->mapping->a_ops->putbackpage)
> + page->mapping->a_ops->putbackpage(page);
> + unlock_page(page);
> + /* drop the extra ref count taken for mobile page isolation */
> + put_page(page);
> +}
Similar comments about the locking, I think the a_ops handler needs to be
responsible. We should not expand the role of the page->lock in the
general case.
> #else
> static inline unsigned long try_to_compact_pages(gfp_t gfp_mask,
> unsigned int order, int alloc_flags,
> @@ -83,6 +150,19 @@ static inline bool compaction_deferred(struct zone *zone, int order)
> return true;
> }
>
> +static inline bool mobile_page(struct page *page)
> +{
> + return false;
> +}
> +
> +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t mode)
> +{
> + return false;
> +}
> +
> +static inline void putback_mobilepage(struct page *page)
> +{
> +}
> #endif /* CONFIG_COMPACTION */
>
> #if defined(CONFIG_COMPACTION) && defined(CONFIG_SYSFS) && defined(CONFIG_NUMA)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index a0653e5..2cc4b24 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -396,6 +396,8 @@ struct address_space_operations {
> */
> int (*migratepage) (struct address_space *,
> struct page *, struct page *, enum migrate_mode);
> + bool (*isolatepage) (struct page *, isolate_mode_t);
> + void (*putbackpage) (struct page *);
> int (*launder_page) (struct page *);
> int (*is_partially_uptodate) (struct page *, unsigned long,
> unsigned long);
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index f34e040..abef145 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -582,6 +582,25 @@ static inline void __ClearPageBalloon(struct page *page)
> atomic_set(&page->_mapcount, -1);
> }
>
> +#define PAGE_MOBILE_MAPCOUNT_VALUE (-255)
> +
> +static inline int PageMobile(struct page *page)
> +{
> + return atomic_read(&page->_mapcount) == PAGE_MOBILE_MAPCOUNT_VALUE;
> +}
> +
> +static inline void __SetPageMobile(struct page *page)
> +{
> + VM_BUG_ON_PAGE(atomic_read(&page->_mapcount) != -1, page);
> + atomic_set(&page->_mapcount, PAGE_MOBILE_MAPCOUNT_VALUE);
> +}
> +
> +static inline void __ClearPageMobile(struct page *page)
> +{
> + VM_BUG_ON_PAGE(!PageMobile(page), page);
> + atomic_set(&page->_mapcount, -1);
> +}
> +
This definition of Mobility would prevent LRU pages ever being considered
"mobile" in the same why. Why do we not either check it's an LRU page (in
which case it's inherently mobile) or has an aops with the correct handlers?
> /*
> * If network-based swap is enabled, sl*b must keep track of whether pages
> * were allocated from pfmemalloc reserves.
> diff --git a/include/uapi/linux/kernel-page-flags.h b/include/uapi/linux/kernel-page-flags.h
> index a6c4962..d50d9e8 100644
> --- a/include/uapi/linux/kernel-page-flags.h
> +++ b/include/uapi/linux/kernel-page-flags.h
> @@ -33,6 +33,7 @@
> #define KPF_THP 22
> #define KPF_BALLOON 23
> #define KPF_ZERO_PAGE 24
> +#define KPF_MOBILE 25
>
>
> #endif /* _UAPILINUX_KERNEL_PAGE_FLAGS_H */
--
Mel Gorman
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 1/4] fs/anon_inodes: new interface to create new inode
From: Mel Gorman @ 2015-07-29 10:50 UTC (permalink / raw)
To: Gioh Kim
Cc: jlayton, bfields, vbabka, iamjoonsoo.kim, viro, mst, koct9i,
minchan, aquini, linux-fsdevel, virtualization, linux-kernel,
linux-api, linux-mm, dri-devel, akpm, Gioh Kim
In-Reply-To: <1436776519-17337-2-git-send-email-gioh.kim@lge.com>
On Mon, Jul 13, 2015 at 05:35:16PM +0900, Gioh Kim wrote:
> From: Gioh Kim <gurugio@hanmail.net>
>
> The anon_inodes has already complete interfaces to create manage
> many anonymous inodes but don't have interface to get
> new inode. Other sub-modules can create anonymous inode
> without creating and mounting it's own pseudo filesystem.
>
> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> Acked-by: Rafael Aquini <aquini@redhat.com>
This is my first run through the series so I'm going to miss details but
this patch confuses me a little. You create an inode to associate with
the balloon dev_info so that page->mapping can be assigned. It's only the
mapping you care about for the aops so why are multiple inodes required? A
driver should be able to share and reference count a single inode. The
motivation to do it that way would be to reduce memory consumption and
this series is motivated by embedded platforms.
anon_inode_getfd has the following
* Creates a new file by hooking it on a single inode. This is useful for files
* that do not need to have a full-fledged inode in order to operate correctly.
* All the files created with anon_inode_getfd() will share a single inode,
* hence saving memory and avoiding code duplication for the file/inode/dentry
* setup. Returns new descriptor or an error code.
If all we care about the inode is the aops then it would follow that
anon_inode_getfd() is ideal. The tradeoff is reference counting overhead.
The changelog needs to explain why anon_inode_getfd() cannot be used.
--
Mel Gorman
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH V5 0/7] Allow user to request memory to be locked on page fault
From: Vlastimil Babka @ 2015-07-29 10:49 UTC (permalink / raw)
To: Michal Hocko, Eric B Munson
Cc: Andrew Morton, Shuah Khan, Michael Kerrisk, Jonathan Corbet,
Ralf Baechle, linux-alpha, linux-kernel, linux-mips, linux-parisc,
linuxppc-dev, sparclinux, linux-xtensa, linux-mm, linux-arch,
linux-api
In-Reply-To: <20150729104532.GE15801@dhcp22.suse.cz>
On 07/29/2015 12:45 PM, Michal Hocko wrote:
>> In a much less
>> likely corner case, it is not possible in the current setup to request
>> all current VMAs be VM_LOCKONFAULT and all future be VM_LOCKED.
>
> Vlastimil has already pointed that out. MCL_FUTURE doesn't clear
> MCL_CURRENT. I was quite surprised in the beginning but it makes a
> perfect sense. mlockall call shouldn't lead into munlocking, that would
> be just weird. Clearing MCL_FUTURE on MCL_CURRENT makes sense on the
> other hand because the request is explicit about _current_ memory and it
> doesn't lead to any munlocking.
Yeah after more thinking it does make some sense despite the perceived
inconsistency, but it's definitely worth documenting properly. It also already
covers the usecase for munlockall2(MCL_FUTURE) which IIRC you had in the earlier
revisions...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 0/4] enable migration of driver pages
From: Mel Gorman @ 2015-07-29 10:49 UTC (permalink / raw)
To: Gioh Kim
Cc: jlayton, bfields, vbabka, iamjoonsoo.kim, viro, mst, koct9i,
minchan, aquini, linux-fsdevel, virtualization, linux-kernel,
linux-api, linux-mm, dri-devel, akpm, Gioh Kim
In-Reply-To: <1436776519-17337-1-git-send-email-gioh.kim@lge.com>
On Mon, Jul 13, 2015 at 05:35:15PM +0900, Gioh Kim wrote:
> My ARM-based platform occured severe fragmentation problem after long-term
> (several days) test. Sometimes even order-3 page allocation failed. It has
> memory size 512MB ~ 1024MB. 30% ~ 40% memory is consumed for graphic processing
> and 20~30 memory is reserved for zram.
>
The primary motivation of this series is to reduce fragmentation by allowing
more kernel pages to be moved. Conceptually that is a worthwhile goal but
there should be at least one major in-kernel user and while balloon
pages were a good starting point, I think we really need to see what the
zram changes look like at the same time.
> I found that many pages of GPU driver and zram are non-movable pages. So I
> reported Minchan Kim, the maintainer of zram, and he made the internal
> compaction logic of zram. And I made the internal compaction of GPU driver.
>
I am not familiar with the internals of zram but I took a look at what
it merged. At a glance the compaction it implements and what you need are
are different in important respects. The core ability to move a zsmalloc
object is useful but the motivation of zram compaction appears to be
reducing the memory footprint. You need to reduce fragmentation which is
not the same. You could be faced with a situation where a full page in an
awkward place. Then there are three choices I can think of quickly and
probably more
1. You can move the whole page to another whole page and update all the
references. This would play nicely with how compactions migrate and
free scanner operates. However, you need free memory to move it
2. You could try moving the full page into other zsmalloc pages so that
memory usage is also potentially reduced. This would work better with
what Minchan intended but then there is the problem of discovery.
Potentially it means though that another address space callback is
required to nominate a target migration page
3. Hybrid approach. First trigger the zsmalloc compaction as it
currently exists, then kick of compaction and move whole pages
regardless of their content. The downside here is that it's expensive
and potentially copies data multiple times but it's going to be
easier to implement than 2.
1 would be the logical starting point, 3 is probably most effective even
if it's expensive and 2 is probably the best overall if the search costs
can be controlled.
This is a lot more complex than what balloon requires which is why I
would like to see it pinned down before new address_space operations are
created. Once they are created and drivers start using them then we lose
a lot of flexibilty and fixing the design becomes a lot harder.
With that in mind, I'll still read the rest of the series.
--
Mel Gorman
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH V5 0/7] Allow user to request memory to be locked on page fault
From: Michal Hocko @ 2015-07-29 10:45 UTC (permalink / raw)
To: Eric B Munson
Cc: Vlastimil Babka, Andrew Morton, Shuah Khan, Michael Kerrisk,
Jonathan Corbet, Ralf Baechle, linux-alpha, linux-kernel,
linux-mips, linux-parisc, linuxppc-dev, sparclinux, linux-xtensa,
linux-mm, linux-arch, linux-api
In-Reply-To: <20150728134942.GB2407@akamai.com>
On Tue 28-07-15 09:49:42, Eric B Munson wrote:
> On Tue, 28 Jul 2015, Michal Hocko wrote:
>
> > [I am sorry but I didn't get to this sooner.]
> >
> > On Mon 27-07-15 10:54:09, Eric B Munson wrote:
> > > Now that VM_LOCKONFAULT is a modifier to VM_LOCKED and
> > > cannot be specified independentally, it might make more sense to mirror
> > > that relationship to userspace. Which would lead to soemthing like the
> > > following:
> >
> > A modifier makes more sense.
> >
> > > To lock and populate a region:
> > > mlock2(start, len, 0);
> > >
> > > To lock on fault a region:
> > > mlock2(start, len, MLOCK_ONFAULT);
> > >
> > > If LOCKONFAULT is seen as a modifier to mlock, then having the flags
> > > argument as 0 mean do mlock classic makes more sense to me.
> > >
> > > To mlock current on fault only:
> > > mlockall(MCL_CURRENT | MCL_ONFAULT);
> > >
> > > To mlock future on fault only:
> > > mlockall(MCL_FUTURE | MCL_ONFAULT);
> > >
> > > To lock everything on fault:
> > > mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT);
> >
> > Makes sense to me. The only remaining and still tricky part would be
> > the munlock{all}(flags) behavior. What should munlock(MLOCK_ONFAULT)
> > do? Keep locked and poppulate the range or simply ignore the flag an
> > just unlock?
> >
> > I can see some sense to allow munlockall(MCL_FUTURE[|MLOCK_ONFAULT]),
> > munlockall(MCL_CURRENT) resp. munlockall(MCL_CURRENT|MCL_FUTURE) but
> > other combinations sound weird to me.
> >
> > Anyway munlock with flags opens new doors of trickiness.
>
> In the current revision there are no new munlock[all] system calls
> introduced. munlockall() unconditionally cleared both MCL_CURRENT and
> MCL_FUTURE before the set and now unconditionally clears all three.
> munlock() does the same for VM_LOCK and VM_LOCKONFAULT.
OK if new munlock{all}(flags) is not introduced then this is much saner
IMO.
> If the user
> wants to adjust mlockall flags today, they need to call mlockall a
> second time with the new flags, this remains true for mlockall after
> this set and the same behavior is mirrored in mlock2.
OK, this makes sense to me.
> The only
> remaining question I have is should we have 2 new mlockall flags so that
> the caller can explicitly set VM_LOCKONFAULT in the mm->def_flags vs
> locking all current VMAs on fault. I ask because if the user wants to
> lock all current VMAs the old way, but all future VMAs on fault they
> have to call mlockall() twice:
>
> mlockall(MCL_CURRENT);
> mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT);
>
> This has the side effect of converting all the current VMAs to
> VM_LOCKONFAULT, but because they were all made present and locked in the
> first call, this should not matter in most cases.
I think this is OK (worth documenting though) considering that ONFAULT
is just modifier for the current mlock* operation. The memory is locked
the same way for both - aka once the memory is present you do not know
whether it was done during mlock call or later during the fault.
> The catch is that,
> like mmap(MAP_LOCKED), mlockall() does not communicate if mm_populate()
> fails. This has been true of mlockall() from the beginning so I don't
> know if it needs more than an entry in the man page to clarify (which I
> will add when I add documentation for MCL_ONFAULT).
Yes this is true but unlike mmap it seems fixable I guess. We do not have
to unmap and we can downgrade mmap_sem to read and the fault so nobody
can race with a concurent mlock.
> In a much less
> likely corner case, it is not possible in the current setup to request
> all current VMAs be VM_LOCKONFAULT and all future be VM_LOCKED.
Vlastimil has already pointed that out. MCL_FUTURE doesn't clear
MCL_CURRENT. I was quite surprised in the beginning but it makes a
perfect sense. mlockall call shouldn't lead into munlocking, that would
be just weird. Clearing MCL_FUTURE on MCL_CURRENT makes sense on the
other hand because the request is explicit about _current_ memory and it
doesn't lead to any munlocking.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH 06/14] kthread: Add kthread_worker_created()
From: Petr Mladek @ 2015-07-29 10:07 UTC (permalink / raw)
To: Tejun Heo
Cc: Andrew Morton, Oleg Nesterov, Ingo Molnar, Peter Zijlstra,
Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Vlastimil Babka,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150728172657.GC5322-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
On Tue 2015-07-28 13:26:57, Tejun Heo wrote:
> Hello,
>
> On Tue, Jul 28, 2015 at 04:39:23PM +0200, Petr Mladek wrote:
> > I would like to make cleaner kthread worker API and hide the definition
> > of struct kthread_worker. It will prevent any custom hacks and make
> > the API more secure.
> >
> > This patch provides an API to check if the worker has been created
> > and hides the implementation details.
>
> Maybe it'd be a better idea to make create_kthread_worker() allocate
> and return pointer to struct kthread_worker? You're adding
> create/destroy interface anyway, it won't need a separate created
> query function and the synchronization rules would be self-evident.
Makes sense. I actually did it this way in one temporary version and reverted
it from some ugly reason.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH 03/14] kthread: Add drain_kthread_worker()
From: Petr Mladek @ 2015-07-29 10:04 UTC (permalink / raw)
To: Tejun Heo
Cc: Andrew Morton, Oleg Nesterov, Ingo Molnar, Peter Zijlstra,
Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Vlastimil Babka,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150728171822.GA5322-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
On Tue 2015-07-28 13:18:22, Tejun Heo wrote:
> Hello,
>
> On Tue, Jul 28, 2015 at 04:39:20PM +0200, Petr Mladek wrote:
> > +/*
> > + * Test whether @work is being queued from another work
> > + * executing on the same kthread.
> > + */
> > +static bool is_chained_work(struct kthread_worker *worker)
> > +{
> > + struct kthread_worker *current_worker;
> > +
> > + current_worker = current_kthread_worker();
> > + /*
> > + * Return %true if I'm a kthread worker executing a work item on
> > + * the given @worker.
> > + */
> > + return current_worker && current_worker == worker;
> > +}
>
> I'm not sure full-on chained work detection is necessary here.
> kthread worker's usages tend to be significantly simpler and draining
> is only gonna be used for destruction.
I think that it might be useful to detect bugs when someone
depends on the worker when it is being destroyed. For example,
I tried to convert "khubd" kthread and there was not easy to
double check that this worked as expected.
I actually think about replacing
WARN_ON_ONCE(!is_chained_work(worker)))
with
WARN_ON(!is_chained_work(worker)))
in queue_kthread_work, so that we get the warning for all misused
workers.
> > +void drain_kthread_worker(struct kthread_worker *worker)
> > +{
> > + int flush_cnt = 0;
> > +
> > + spin_lock_irq(&worker->lock);
> > + worker->nr_drainers++;
> > +
> > + while (!list_empty(&worker->work_list)) {
> > + /*
> > + * Unlock, so we could move forward. Note that queuing
> > + * is limited by @nr_drainers > 0.
> > + */
> > + spin_unlock_irq(&worker->lock);
> > +
> > + flush_kthread_worker(worker);
> > +
> > + if (++flush_cnt == 10 ||
> > + (flush_cnt % 100 == 0 && flush_cnt <= 1000))
> > + pr_warn("kthread worker %s: drain_kthread_worker() isn't complete after %u tries\n",
> > + worker->task->comm, flush_cnt);
> > +
> > + spin_lock_irq(&worker->lock);
> > + }
>
> I'd just do something like WARN_ONCE(flush_cnt++ > 10, "kthread worker: ...").
This would print the warning only for one broken worker. But I do not
have strong opinion about it.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v9 0/9] Add simple NVMEM Framework via regmap.
From: Srinivas Kandagatla @ 2015-07-29 8:55 UTC (permalink / raw)
To: Rajendra Nayak, khilman, linux-arm-kernel, Greg Kroah-Hartman
Cc: Rob Herring, Mark Brown, s.hauer, linux-api, linux-kernel,
devicetree, linux-arm-msm, arnd, sboyd, pantelis.antoniou,
mporter, stefan.wahren, wxt
In-Reply-To: <55B893A7.2030206@codeaurora.org>
On 29/07/15 09:49, Rajendra Nayak wrote:
> On 07/27/2015 04:42 PM, Srinivas Kandagatla wrote:
>> Hi Greg/Kevin,
>>
>> This patchset adds a new simple NVMEM framework to kernel, and it is
>> tested
>> with various drivers like "QCOM thermal sensors", "QCOM cpr driver",
>> "begal bone cape manager" and few more on the way.
>>
>> Can you please consider this as 4.3 material, AFAIK there are more
>> than 3 drivers
>> depending on this framework which are wating since last 2 merge windows.
>
> I have been testing these with the qcom tsens driver, and did test v9
> as well, so feel free to add my tested by for the entire series,
>
> Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Thanks Rajendra for the tested-by.
^ permalink raw reply
* Re: [PATCH v9 0/9] Add simple NVMEM Framework via regmap.
From: Rajendra Nayak @ 2015-07-29 8:49 UTC (permalink / raw)
To: Srinivas Kandagatla, khilman-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Greg Kroah-Hartman
Cc: Rob Herring, Mark Brown, s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w,
mporter-OWPKS81ov/FWk0Htik3J/w, stefan.wahren-eS4NqCHxEME,
wxt-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <1437995567-11203-1-git-send-email-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On 07/27/2015 04:42 PM, Srinivas Kandagatla wrote:
> Hi Greg/Kevin,
>
> This patchset adds a new simple NVMEM framework to kernel, and it is tested
> with various drivers like "QCOM thermal sensors", "QCOM cpr driver",
> "begal bone cape manager" and few more on the way.
>
> Can you please consider this as 4.3 material, AFAIK there are more than 3 drivers
> depending on this framework which are wating since last 2 merge windows.
I have been testing these with the qcom tsens driver, and did test v9
as well, so feel free to add my tested by for the entire series,
Tested-by: Rajendra Nayak <rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
^ permalink raw reply
* Re: [PATCH v7 4/6] block: loop: prepare for supporing direct IO
From: Dave Chinner @ 2015-07-29 8:41 UTC (permalink / raw)
To: Ming Lei
Cc: Christoph Hellwig, Jens Axboe, Linux Kernel Mailing List,
Justin M. Forbes, Jeff Moyer, Tejun Heo, linux-api
In-Reply-To: <CACVXFVMkU0TLxys_e3arGChdmgM-P2zQhPBfYx5kUXGbUtd_8A@mail.gmail.com>
On Wed, Jul 29, 2015 at 03:33:52AM -0400, Ming Lei wrote:
> On Mon, Jul 27, 2015 at 1:33 PM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Mon, Jul 27, 2015 at 05:53:33AM -0400, Ming Lei wrote:
> >> Because size has to be 4k aligned too.
> >
> > Yes. But again I don't see any reason to limit us to a hardcoded 512
> > byte block size here, especially considering the patches to finally
>
> From loop block's view, the request size can be any count of 512-byte
> sectors, then the transfer size to backing device can't guarantee to be
> 4k aligned always.
In theory, yes. In practise, doesn't happen very often.
> > allow enabling other block sizes from userspace.
>
> I have some questions about the patchset, and looks the author doesn't
> reply it yet.
>
> On Mon, Jul 27, 2015 at 6:06 PM, Dave Chinner <david@fromorbit.com> wrote:
> >> Because size has to be 4k aligned too.
> >
> > So check that, too. Any >= 4k block size filesystem should be doing
> > mostly 4k aligned and sized IO...
>
> I guess you mean we only use direct IO for the 4k aligned and sized IO?
> If so, that won't be efficient because the page cache has to be flushed
> during the switch.
It will be extremely rare for a 4k block size filesystem to do
anything other than 4k aligned and sized IO. Think about it for a
minute: what does the page cache do to unaligned IO patterns (i.e.
buffered IO)? It does IO in page sizes, and so if the application
if doing badly aligned or sized IO with buffered IO, then the
underlying device will only ever size page sized and aligned IO.
Hence sector aligned IO will only come from applications doing
direct IO. If the application is doing direct IO and it's not
properly aligned, then it already is going to get sucky performance
because most filesystem serialise sub-block size direct IO because
concurrent sub-block IOs to the same block usually leads to data
corruption.
So, really, sector aligned/sized direct IO is a sucky performance
path before we even get to the loop device, so we don't really need
to care how fast the loop device handles this case. The loop device
just needs to ensure that it doesn't corrupt data when badly aligned
IOs come in... ;)
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [PATCH v7 4/6] block: loop: prepare for supporing direct IO
From: Ming Lei @ 2015-07-29 7:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Dave Chinner, Jens Axboe, Linux Kernel Mailing List,
Justin M. Forbes, Jeff Moyer, Tejun Heo, linux-api
In-Reply-To: <20150727173331.GA17594@infradead.org>
On Mon, Jul 27, 2015 at 1:33 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Jul 27, 2015 at 05:53:33AM -0400, Ming Lei wrote:
>> Because size has to be 4k aligned too.
>
> Yes. But again I don't see any reason to limit us to a hardcoded 512
> byte block size here, especially considering the patches to finally
>From loop block's view, the request size can be any count of 512-byte
sectors, then the transfer size to backing device can't guarantee to be
4k aligned always.
> allow enabling other block sizes from userspace.
I have some questions about the patchset, and looks the author doesn't
reply it yet.
On Mon, Jul 27, 2015 at 6:06 PM, Dave Chinner <david@fromorbit.com> wrote:
>> Because size has to be 4k aligned too.
>
> So check that, too. Any >= 4k block size filesystem should be doing
> mostly 4k aligned and sized IO...
I guess you mean we only use direct IO for the 4k aligned and sized IO?
If so, that won't be efficient because the page cache has to be flushed
during the switch.
Thanks,
Ming
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH v3 01/11] stm class: Introduce an abstraction for System Trace Module devices
From: Chunyan Zhang @ 2015-07-29 4:21 UTC (permalink / raw)
To: Alexander Shishkin
Cc: Greg Kroah-Hartman,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Mathieu Poirier, peter.lachner-ral2JQCrhuEAvxtiuMwx3w,
norbert.schulz-ral2JQCrhuEAvxtiuMwx3w,
keven.boell-ral2JQCrhuEAvxtiuMwx3w,
yann.fouassier-ral2JQCrhuEAvxtiuMwx3w,
laurent.fert-ral2JQCrhuEAvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Chunyan Zhang,
Mark Brown
In-Reply-To: <1436177344-16751-2-git-send-email-alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On Mon, Jul 6, 2015 at 6:08 PM, Alexander Shishkin
<alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> A System Trace Module (STM) is a device exporting data in System Trace
> Protocol (STP) format as defined by MIPI STP standards. Examples of such
> devices are Intel Trace Hub and Coresight STM.
>
> This abstraction provides a unified interface for software trace sources
> to send their data over an STM device to a debug host. In order to do
> that, such a trace source needs to be assigned a pair of master/channel
> identifiers that all the data from this source will be tagged with. The
> STP decoder on the debug host side will use these master/channel tags to
> distinguish different trace streams from one another inside one STP
> stream.
>
> This abstraction provides a configfs-based policy management mechanism
> for dynamic allocation of these master/channel pairs based on trace
> source-supplied string identifier. It has the flexibility of being
> defined at runtime and at the same time (provided that the policy
> definition is aligned with the decoding end) consistency.
>
> For userspace trace sources, this abstraction provides write()-based and
> mmap()-based (if the underlying stm device allows this) output mechanism.
>
> For kernel-side trace sources, we provide "stm_source" device class that
> can be connected to an stm device at run time.
>
> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
> Documentation/ABI/testing/configfs-stp-policy | 48 +
> Documentation/ABI/testing/sysfs-class-stm | 14 +
> Documentation/ABI/testing/sysfs-class-stm_source | 11 +
> Documentation/ioctl/ioctl-number.txt | 3 +
> Documentation/trace/stm.txt | 80 ++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/hwtracing/stm/Kconfig | 8 +
> drivers/hwtracing/stm/Makefile | 3 +
> drivers/hwtracing/stm/core.c | 1029 ++++++++++++++++++++++
> drivers/hwtracing/stm/policy.c | 529 +++++++++++
> drivers/hwtracing/stm/stm.h | 87 ++
> include/linux/stm.h | 126 +++
> include/uapi/linux/stm.h | 50 ++
> 14 files changed, 1991 insertions(+)
> create mode 100644 Documentation/ABI/testing/configfs-stp-policy
> create mode 100644 Documentation/ABI/testing/sysfs-class-stm
> create mode 100644 Documentation/ABI/testing/sysfs-class-stm_source
> create mode 100644 Documentation/trace/stm.txt
> create mode 100644 drivers/hwtracing/stm/Kconfig
> create mode 100644 drivers/hwtracing/stm/Makefile
> create mode 100644 drivers/hwtracing/stm/core.c
> create mode 100644 drivers/hwtracing/stm/policy.c
> create mode 100644 drivers/hwtracing/stm/stm.h
> create mode 100644 include/linux/stm.h
> create mode 100644 include/uapi/linux/stm.h
>
> diff --git a/Documentation/ABI/testing/configfs-stp-policy b/Documentation/ABI/testing/configfs-stp-policy
> new file mode 100644
> index 0000000000..421ce6825c
> --- /dev/null
> +++ b/Documentation/ABI/testing/configfs-stp-policy
> @@ -0,0 +1,48 @@
> +What: /config/stp-policy
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + This group contains policies mandating Master/Channel allocation
> + for software sources wishing to send trace data over an STM
> + device.
> +
> +What: /config/stp-policy/<device>.<policy>
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + This group is the root of a policy; its name is a concatenation
> + of an stm device name to which this policy applies and an
> + arbitrary string. If <device> part doesn't match an existing
> + stm device, mkdir will fail with ENODEV; if that device already
> + has a policy assigned to it, mkdir will fail with EBUSY.
> +
> +What: /config/stp-policy/<device>.<policy>/device
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + STM device to which this policy applies, read only. Same as the
> + <device> component of its parent directory.
> +
> +What: /config/stp-policy/<device>.<policy>/<node>
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + Policy node is a string identifier that software clients will
> + use to request a master/channel to be allocated and assigned to
> + them.
> +
> +What: /config/stp-policy/<device>.<policy>/<node>/masters
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + Range of masters from which to allocate for users of this node.
> + Write two numbers: the first master and the last master number.
> +
> +What: /config/stp-policy/<device>.<policy>/<node>/channels
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + Range of channels from which to allocate for users of this node.
> + Write two numbers: the first channel and the last channel
> + number.
> +
> diff --git a/Documentation/ABI/testing/sysfs-class-stm b/Documentation/ABI/testing/sysfs-class-stm
> new file mode 100644
> index 0000000000..c9aa4f3fc9
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-stm
> @@ -0,0 +1,14 @@
> +What: /sys/class/stm/<stm>/masters
> +Date: June 2015
> +KernelVersion: 4.3
> +Contact: Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> +Description:
> + Shows first and last available to software master numbers on
> + this STM device.
> +
> +What: /sys/class/stm/<stm>/channels
> +Date: June 2015
> +KernelVersion: 4.3
> +Contact: Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> +Description:
> + Shows the number of channels per master on this STM device.
> diff --git a/Documentation/ABI/testing/sysfs-class-stm_source b/Documentation/ABI/testing/sysfs-class-stm_source
> new file mode 100644
> index 0000000000..57b8dd39bb
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-stm_source
> @@ -0,0 +1,11 @@
> +What: /sys/class/stm_source/<stm_source>/stm_source_link
> +Date: June 2015
> +KernelVersion: 4.3
> +Contact: Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> +Description:
> + stm_source device linkage to stm device, where its tracing data
> + is directed. Reads return an existing connection or "<none>" if
> + this stm_source is not connected to any stm device yet.
> + Write an existing (registered) stm device's name here to
> + connect that device. If a device is already connected to this
> + stm_source, it will first be disconnected.
> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
> index 611c52267d..7fa29b4ece 100644
> --- a/Documentation/ioctl/ioctl-number.txt
> +++ b/Documentation/ioctl/ioctl-number.txt
> @@ -81,6 +81,9 @@ Code Seq#(hex) Include File Comments
> 0x22 all scsi/sg.h
> '#' 00-3F IEEE 1394 Subsystem Block for the entire subsystem
> '$' 00-0F linux/perf_counter.h, linux/perf_event.h
> +'%' 00-0F include/uapi/linux/stm.h
> + System Trace Module subsystem
> + <mailto:alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> '&' 00-07 drivers/firewire/nosy-user.h
> '1' 00-1F <linux/timepps.h> PPS kit from Ulrich Windl
> <ftp://ftp.de.kernel.org/pub/linux/daemons/ntp/PPS/>
> diff --git a/Documentation/trace/stm.txt b/Documentation/trace/stm.txt
> new file mode 100644
> index 0000000000..e140f2e8d7
> --- /dev/null
> +++ b/Documentation/trace/stm.txt
> @@ -0,0 +1,80 @@
> +System Trace Module
> +===================
> +
> +System Trace Module (STM) is a device described in MIPI STP specs as
> +STP trace stream generator. STP (System Trace Protocol) is a trace
> +protocol multiplexing data from multiple trace sources, each one of
> +which is assigned a unique pair of master and channel. While some of
> +these masters and channels are statically allocated to certain
> +hardware trace sources, others are available to software. Software
> +trace sources are usually free to pick for themselves any
> +master/channel combination from this pool.
> +
> +On the receiving end of this STP stream (the decoder side), trace
> +sources can only be identified by master/channel combination, so in
> +order for the decoder to be able to make sense of the trace that
> +involves multiple trace sources, it needs to be able to map those
> +master/channel pairs to the trace sources that it understands.
> +
> +For instance, it is helpful to know that syslog messages come on
> +master 7 channel 15, while arbitrary user applications can use masters
> +48 to 63 and channels 0 to 127.
> +
> +To solve this mapping problem, stm class provides a policy management
> +mechanism via configfs, that allows defining rules that map string
> +identifiers to ranges of masters and channels. If these rules (policy)
> +are consistent with what decoder expects, it will be able to properly
> +process the trace data.
> +
> +This policy is a tree structure containing rules (policy_node) that
> +have a name (string identifier) and a range of masters and channels
> +associated with it, located in "stp-policy" subsystem directory in
> +configfs. The topmost directory's name (the policy) is formatted as
> +the STM device name to which this policy applies and and arbitrary
> +string identifier separated by a stop. From the examle above, a rule
> +may look like this:
> +
> +$ ls /config/stp-policy/dummy_stm.my-policy/user
> +channels masters
> +$ cat /config/stp-policy/dummy_stm.my-policy/user/masters
> +48 63
> +$ cat /config/stp-policy/dummy_stm.my-policy/user/channels
> +0 127
> +
> +which means that the master allocation pool for this rule consists of
> +masters 48 through 63 and channel allocation pool has channels 0
> +through 127 in it. Now, any producer (trace source) identifying itself
> +with "user" identification string will be allocated a master and
> +channel from within these ranges.
> +
> +These rules can be nested, for example, one can define a rule "dummy"
> +under "user" directory from the example above and this new rule will
> +be used for trace sources with the id string of "user/dummy".
> +
> +Trace sources have to open the stm class device's node and write their
> +trace data into its file descriptor. In order to identify themselves
> +to the policy, they need to do a STP_POLICY_ID_SET ioctl on this file
> +descriptor providing their id string. Otherwise, they will be
> +automatically allocated a master/channel pair upon first write to this
> +file descriptor according to the "default" rule of the policy, if such
> +exists.
> +
> +Some STM devices may allow direct mapping of the channel mmio regions
> +to userspace for zero-copy writing. One mappable page (in terms of
> +mmu) will usually contain multiple channels' mmios, so the user will
> +need to allocate that many channels to themselves (via the
> +aforementioned ioctl() call) to be able to do this. That is, if your
> +stm device's channel mmio region is 64 bytes and hardware page size is
> +4096 bytes, after a successful STP_POLICY_ID_SET ioctl() call with
> +width==64, you should be able to mmap() one page on this file
> +descriptor and obtain direct access to an mmio region for 64 channels.
> +
> +For kernel-based trace sources, there is "stm_source" device
> +class. Devices of this class can be connected and disconnected to/from
> +stm devices at runtime via a sysfs attribute.
> +
> +Examples of STM devices are Intel Trace Hub [1] and Coresight STM
> +[2].
> +
> +[1] https://software.intel.com/sites/default/files/managed/d3/3c/intel-th-developer-manual.pdf
> +[2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0444b/index.html
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 6e973b8e3a..96efe4522c 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -184,4 +184,6 @@ source "drivers/android/Kconfig"
>
> source "drivers/nvdimm/Kconfig"
>
> +source "drivers/hwtracing/stm/Kconfig"
> +
> endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index b64b49f6e0..6647bc923f 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -164,4 +164,5 @@ obj-$(CONFIG_MCB) += mcb/
> obj-$(CONFIG_RAS) += ras/
> obj-$(CONFIG_THUNDERBOLT) += thunderbolt/
> obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
> +obj-$(CONFIG_STM) += hwtracing/stm/
> obj-$(CONFIG_ANDROID) += android/
> diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
> new file mode 100644
> index 0000000000..90ed327461
> --- /dev/null
> +++ b/drivers/hwtracing/stm/Kconfig
> @@ -0,0 +1,8 @@
> +config STM
> + tristate "System Trace Module devices"
> + help
> + A System Trace Module (STM) is a device exporting data in System
> + Trace Protocol (STP) format as defined by MIPI STP standards.
> + Examples of such devices are Intel Trace Hub and Coresight STM.
> +
> + Say Y here to enable System Trace Module device support.
> diff --git a/drivers/hwtracing/stm/Makefile b/drivers/hwtracing/stm/Makefile
> new file mode 100644
> index 0000000000..adec701649
> --- /dev/null
> +++ b/drivers/hwtracing/stm/Makefile
> @@ -0,0 +1,3 @@
> +obj-$(CONFIG_STM) += stm_core.o
> +
> +stm_core-y := core.o policy.o
> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
> new file mode 100644
> index 0000000000..b79c42c625
> --- /dev/null
> +++ b/drivers/hwtracing/stm/core.c
> @@ -0,0 +1,1029 @@
> +/*
> + * System Trace Module (STM) infrastructure
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * STM class implements generic infrastructure for System Trace Module devices
> + * as defined in MIPI STPv2 specification.
> + */
> +
> +#include <linux/uaccess.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/compat.h>
> +#include <linux/kdev_t.h>
> +#include <linux/srcu.h>
> +#include <linux/slab.h>
> +#include <linux/stm.h>
> +#include <linux/fs.h>
> +#include <linux/mm.h>
> +#include "stm.h"
> +
> +#include <uapi/linux/stm.h>
> +
> +static unsigned int stm_core_up;
> +
> +/*
> + * The SRCU here makes sure that STM device doesn't disappear from under a
> + * stm_source_write() caller, which may want to have as little overhead as
> + * possible.
> + */
> +static struct srcu_struct stm_source_srcu;
> +
> +static ssize_t masters_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct stm_device *stm = to_stm_device(dev);
> + int ret;
> +
> + ret = sprintf(buf, "%u %u\n", stm->data->sw_start, stm->data->sw_end);
> +
> + return ret;
> +}
> +
> +static DEVICE_ATTR_RO(masters);
> +
> +static ssize_t channels_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct stm_device *stm = to_stm_device(dev);
> + int ret;
> +
> + ret = sprintf(buf, "%u\n", stm->data->sw_nchannels);
> +
> + return ret;
> +}
> +
> +static DEVICE_ATTR_RO(channels);
> +
> +static struct attribute *stm_attrs[] = {
> + &dev_attr_masters.attr,
> + &dev_attr_channels.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(stm);
> +
> +static struct class stm_class = {
> + .name = "stm",
> + .dev_groups = stm_groups,
> +};
> +
> +static int stm_dev_match(struct device *dev, const void *data)
> +{
> + const char *name = data;
> +
> + return sysfs_streq(name, dev_name(dev));
> +}
> +
> +/**
> + * stm_find_device() - find stm device by name
> + * @buf: character buffer containing the name
> + *
> + * This is called when either policy gets assigned to an stm device or an
> + * stm_source device gets linked to an stm device.
> + *
> + * This grabs device's reference (get_device()) and module reference, both
> + * of which the calling path needs to make sure to drop with stm_put_device().
> + *
> + * Return: stm device pointer or null if lookup failed.
> + */
> +struct stm_device *stm_find_device(const char *buf)
> +{
> + struct stm_device *stm;
> + struct device *dev;
> +
> + if (!stm_core_up)
> + return NULL;
> +
> + dev = class_find_device(&stm_class, NULL, buf, stm_dev_match);
> + if (!dev)
> + return NULL;
> +
> + stm = to_stm_device(dev);
> + if (!try_module_get(stm->owner)) {
> + put_device(dev);
> + return NULL;
> + }
> +
> + return stm;
> +}
> +
> +/**
> + * stm_put_device() - drop references on the stm device
> + * @stm: stm device, previously acquired by stm_find_device()
> + *
> + * This drops the module reference and device reference taken by
> + * stm_find_device().
> + */
> +void stm_put_device(struct stm_device *stm)
> +{
> + module_put(stm->owner);
> + put_device(&stm->dev);
> +}
> +
> +/*
> + * Internally we only care about software-writable masters here, that is the
> + * ones in the range [stm_data->sw_start..stm_data..sw_end], however we need
> + * original master numbers to be visible externally, since they are the ones
> + * that will appear in the STP stream. Thus, the internal bookkeeping uses
> + * $master - stm_data->sw_start to reference master descriptors and such.
> + */
> +
> +#define __stm_master(_s, _m) \
> + ((_s)->masters[(_m) - (_s)->data->sw_start])
> +
> +static inline struct stp_master *
> +stm_master(struct stm_device *stm, unsigned int idx)
> +{
> + if (idx < stm->data->sw_start || idx > stm->data->sw_end)
> + return NULL;
> +
> + return __stm_master(stm, idx);
> +}
> +
> +static int stp_master_alloc(struct stm_device *stm, unsigned int idx)
> +{
> + struct stp_master *master;
> + size_t size;
> +
> + size = ALIGN(stm->data->sw_nchannels, 8) / 8;
> + size += sizeof(struct stp_master);
> + master = kzalloc(size, GFP_ATOMIC);
> + if (!master)
> + return -ENOMEM;
> +
> + master->nr_free = stm->data->sw_nchannels;
> + __stm_master(stm, idx) = master;
> +
> + return 0;
> +}
> +
> +static void stp_master_free(struct stm_device *stm, unsigned int idx)
> +{
> + struct stp_master *master = stm_master(stm, idx);
> +
> + if (!master)
> + return;
> +
> + __stm_master(stm, idx) = NULL;
> + kfree(master);
> +}
> +
> +static void stm_output_claim(struct stm_device *stm, struct stm_output *output)
> +{
> + struct stp_master *master = stm_master(stm, output->master);
> +
> + if (WARN_ON_ONCE(master->nr_free < output->nr_chans))
> + return;
> +
> + bitmap_allocate_region(&master->chan_map[0], output->channel,
> + ilog2(output->nr_chans));
> +
> + master->nr_free -= output->nr_chans;
> +}
> +
> +static void
> +stm_output_disclaim(struct stm_device *stm, struct stm_output *output)
> +{
> + struct stp_master *master = stm_master(stm, output->master);
> +
> + bitmap_release_region(&master->chan_map[0], output->channel,
> + ilog2(output->nr_chans));
> +
> + output->nr_chans = 0;
> + master->nr_free += output->nr_chans;
> +}
> +
> +/*
> + * This is like bitmap_find_free_region(), except it can ignore @start bits
> + * at the beginning.
> + */
> +static int find_free_channels(unsigned long *bitmap, unsigned int start,
> + unsigned int end, unsigned int width)
> +{
> + unsigned int pos;
> + int i;
> +
> + for (pos = start; pos < end + 1; pos = ALIGN(pos, width)) {
> + pos = find_next_zero_bit(bitmap, end + 1, pos);
> + if (pos + width > end + 1)
> + break;
> +
> + if (pos & (width - 1))
> + continue;
> +
> + for (i = 1; i < width && !test_bit(pos + i, bitmap); i++)
> + ;
> + if (i == width)
> + return pos;
> + }
> +
> + return -1;
> +}
> +
> +static unsigned int
> +stm_find_master_chan(struct stm_device *stm, unsigned int width,
> + unsigned int *mstart, unsigned int mend,
> + unsigned int *cstart, unsigned int cend)
> +{
> + struct stp_master *master;
> + unsigned int midx;
> + int pos, err;
> +
> + for (midx = *mstart; midx <= mend; midx++) {
> + if (!stm_master(stm, midx)) {
> + err = stp_master_alloc(stm, midx);
> + if (err)
> + return err;
> + }
> +
> + master = stm_master(stm, midx);
> +
> + if (!master->nr_free)
> + continue;
> +
> + pos = find_free_channels(master->chan_map, *cstart, cend,
> + width);
> + if (pos < 0)
> + continue;
> +
> + *mstart = midx;
> + *cstart = pos;
> + return 0;
> + }
> +
> + return -ENOSPC;
> +}
> +
> +static int stm_output_assign(struct stm_device *stm, unsigned int width,
> + struct stp_policy_node *policy_node,
> + struct stm_output *output)
> +{
> + unsigned int midx, cidx, mend, cend;
> + int ret = -EINVAL;
> +
> + if (width > stm->data->sw_nchannels)
> + return -EINVAL;
> +
> + if (policy_node) {
> + stp_policy_node_get_ranges(policy_node,
> + &midx, &mend, &cidx, &cend);
> + } else {
> + midx = stm->data->sw_start;
> + cidx = 0;
> + mend = stm->data->sw_end;
> + cend = stm->data->sw_nchannels - 1;
> + }
> +
> + spin_lock(&stm->mc_lock);
> + /* output is already assigned -- shouldn't happen */
> + if (WARN_ON_ONCE(output->nr_chans))
> + goto unlock;
> +
> + ret = stm_find_master_chan(stm, width, &midx, mend, &cidx, cend);
> + if (ret)
> + goto unlock;
> +
> + output->master = midx;
> + output->channel = cidx;
> + output->nr_chans = width;
> + stm_output_claim(stm, output);
> + dev_dbg(&stm->dev, "assigned %u:%u (+%u)\n", midx, cidx, width);
> +
> + ret = 0;
> +unlock:
> + spin_unlock(&stm->mc_lock);
> +
> + return ret;
> +}
> +
> +static void stm_output_free(struct stm_device *stm, struct stm_output *output)
> +{
> + spin_lock(&stm->mc_lock);
> + if (output->nr_chans)
> + stm_output_disclaim(stm, output);
> + spin_unlock(&stm->mc_lock);
> +}
> +
> +static int major_match(struct device *dev, const void *data)
> +{
> + unsigned int major = *(unsigned int *)data;
> +
> + return MAJOR(dev->devt) == major;
> +}
> +
> +static int stm_char_open(struct inode *inode, struct file *file)
> +{
> + struct stm_file *stmf;
> + struct device *dev;
> + unsigned int major = imajor(inode);
> + int err = -ENODEV;
> +
> + dev = class_find_device(&stm_class, NULL, &major, major_match);
> + if (!dev)
> + return -ENODEV;
> +
> + stmf = kzalloc(sizeof(*stmf), GFP_KERNEL);
> + if (!stmf)
> + return -ENOMEM;
> +
> + stmf->stm = to_stm_device(dev);
> +
> + if (!try_module_get(stmf->stm->owner))
> + goto err_free;
> +
> + file->private_data = stmf;
> +
> + return nonseekable_open(inode, file);
> +
> +err_free:
> + kfree(stmf);
> +
> + return err;
> +}
> +
> +static int stm_char_release(struct inode *inode, struct file *file)
> +{
> + struct stm_file *stmf = file->private_data;
> +
> + stm_output_free(stmf->stm, &stmf->output);
> + stm_put_device(stmf->stm);
> + kfree(stmf);
> +
> + return 0;
> +}
> +
> +static int stm_file_assign(struct stm_file *stmf, char *id, unsigned int width)
> +{
> + struct stm_device *stm = stmf->stm;
> + int ret;
> +
> + stmf->policy_node = stp_policy_node_lookup(stm, id);
> +
> + ret = stm_output_assign(stm, width, stmf->policy_node, &stmf->output);
> +
> + if (stmf->policy_node)
> + stp_policy_node_put(stmf->policy_node);
> +
> + return ret;
> +}
> +
> +static void stm_write(struct stm_data *data, unsigned int master,
> + unsigned int channel, const char *buf, size_t count)
> +{
> + unsigned int flags = STP_PACKET_TIMESTAMPED;
> + const unsigned char *p = buf, nil = 0;
> + size_t pos;
> + ssize_t sz;
> +
> + for (pos = 0, p = buf; count > pos; pos += sz, p += sz) {
> + sz = min_t(unsigned int, count - pos, 8);
> + sz = data->packet(data, master, channel, STP_PACKET_DATA, flags,
> + sz, p);
> + flags = 0;
> + }
> +
> + data->packet(data, master, channel, STP_PACKET_FLAG, 0, 0, &nil);
> +}
> +
> +static ssize_t stm_char_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct stm_file *stmf = file->private_data;
> + struct stm_device *stm = stmf->stm;
> + char *kbuf;
> + int err;
> +
> + /*
> + * if no m/c have been assigned to this writer up to this
> + * point, use "default" policy entry
> + */
> + if (!stmf->output.nr_chans) {
> + err = stm_file_assign(stmf, "default", 1);
> + /*
> + * EBUSY means that somebody else just assigned this
> + * output, which is just fine for write()
> + */
> + if (err && err != -EBUSY)
> + return err;
> + }
> +
> + kbuf = kmalloc(count + 1, GFP_KERNEL);
> + if (!kbuf)
> + return -ENOMEM;
> +
> + err = copy_from_user(kbuf, buf, count);
> + if (err) {
> + kfree(kbuf);
> + return -EFAULT;
> + }
> +
> + stm_write(stm->data, stmf->output.master, stmf->output.channel, kbuf,
> + count);
> +
> + kfree(kbuf);
> +
> + return count;
> +}
> +
> +static int stm_char_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct stm_file *stmf = file->private_data;
> + struct stm_device *stm = stmf->stm;
> + unsigned long size, phys;
> +
> + if (!stm->data->mmio_addr)
> + return -EOPNOTSUPP;
> +
> + if (vma->vm_pgoff)
> + return -EINVAL;
> +
> + size = vma->vm_end - vma->vm_start;
> +
> + if (stmf->output.nr_chans * stm->data->sw_mmiosz != size)
> + return -EINVAL;
> +
> + phys = stm->data->mmio_addr(stm->data, stmf->output.master,
> + stmf->output.channel,
> + stmf->output.nr_chans);
> +
> + if (!phys)
> + return -EINVAL;
> +
> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
> + vm_iomap_memory(vma, phys, size);
> +
> + return 0;
> +}
> +
> +static int stm_char_policy_set_ioctl(struct stm_file *stmf, void __user *arg)
> +{
> + struct stm_device *stm = stmf->stm;
> + struct stp_policy_id *id;
> + int ret = -EINVAL;
> + u32 size;
> +
> + if (stmf->output.nr_chans)
> + return -EBUSY;
> +
> + if (copy_from_user(&size, arg, sizeof(size)))
> + return -EFAULT;
> +
> + if (size >= PATH_MAX + sizeof(*id))
> + return -EINVAL;
> +
> + /*
> + * size + 1 to make sure the .id string at the bottom is terminated,
> + * which is also why memdup_user() is not useful here
> + */
> + id = kzalloc(size + 1, GFP_KERNEL);
> + if (!id)
> + return -ENOMEM;
> +
> + if (copy_from_user(id, arg, size)) {
> + ret = -EFAULT;
> + goto err_free;
> + }
> +
> + if (id->__reserved_0 || id->__reserved_1)
> + goto err_free;
> +
> + if (id->width < 1 ||
> + id->width > PAGE_SIZE / stm->data->sw_mmiosz)
> + goto err_free;
> +
> + ret = stm_file_assign(stmf, id->id, id->width);
> + if (ret)
> + goto err_free;
> +
> + ret = 0;
> +
> + if (stm->data->link)
> + ret = stm->data->link(stm->data, stmf->output.master,
> + stmf->output.channel);
> +
> + if (ret) {
> + stm_output_free(stmf->stm, &stmf->output);
> + stm_put_device(stmf->stm);
> + }
> +
> +err_free:
> + kfree(id);
> +
> + return ret;
> +}
> +
> +static int stm_char_policy_get_ioctl(struct stm_file *stmf, void __user *arg)
> +{
> + struct stp_policy_id id = {
> + .size = sizeof(id),
> + .master = stmf->output.master,
> + .channel = stmf->output.channel,
> + .width = stmf->output.nr_chans,
> + .__reserved_0 = 0,
> + .__reserved_1 = 0,
> + };
> +
> + return copy_to_user(arg, &id, id.size) ? -EFAULT : 0;
> +}
> +
> +static long
> +stm_char_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + struct stm_file *stmf = file->private_data;
> + struct stm_data *stm_data = stmf->stm->data;
> + int err = -ENOTTY;
> + u64 options;
> +
> + switch (cmd) {
> + case STP_POLICY_ID_SET:
> + err = stm_char_policy_set_ioctl(stmf, (void __user *)arg);
> + if (err)
> + return err;
> +
> + return stm_char_policy_get_ioctl(stmf, (void __user *)arg);
> +
> + case STP_POLICY_ID_GET:
> + return stm_char_policy_get_ioctl(stmf, (void __user *)arg);
> +
> + case STP_SET_OPTIONS:
> + if (copy_from_user(&options, (u64 __user *)arg, sizeof(u64)))
> + return -EFAULT;
> +
> + if (stm_data->set_options)
> + err = stm_data->set_options(stm_data,
> + stmf->output.master,
> + stmf->output.channel,
> + stmf->output.nr_chans,
> + options);
> +
> + break;
> + default:
> + break;
> + }
> +
> + return err;
> +}
> +
> +#ifdef CONFIG_COMPAT
> +static long
> +stm_char_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + return stm_char_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> +}
> +#else
> +#define stm_char_compat_ioctl NULL
> +#endif
> +
> +static const struct file_operations stm_fops = {
> + .open = stm_char_open,
> + .release = stm_char_release,
> + .write = stm_char_write,
> + .mmap = stm_char_mmap,
> + .unlocked_ioctl = stm_char_ioctl,
> + .compat_ioctl = stm_char_compat_ioctl,
> + .llseek = no_llseek,
> +};
> +
> +static void stm_device_release(struct device *dev)
> +{
> + struct stm_device *stm = to_stm_device(dev);
> +
> + kfree(stm);
> +}
> +
> +int stm_register_device(struct device *parent, struct stm_data *stm_data,
> + struct module *owner)
> +{
> + struct stm_device *stm;
> + unsigned int nmasters;
> + int err = -ENOMEM;
> +
> + if (!stm_core_up)
> + return -EPROBE_DEFER;
> +
> + if (!stm_data->packet || !stm_data->sw_nchannels)
> + return -EINVAL;
> +
> + nmasters = stm_data->sw_end - stm_data->sw_start;
> + stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
> + if (!stm)
> + return -ENOMEM;
> +
> + stm->major = register_chrdev(0, stm_data->name, &stm_fops);
> + if (stm->major < 0)
> + goto err_free;
> +
> + device_initialize(&stm->dev);
> + stm->dev.devt = MKDEV(stm->major, 0);
> + stm->dev.class = &stm_class;
> + stm->dev.parent = parent;
> + stm->dev.release = stm_device_release;
> +
> + err = kobject_set_name(&stm->dev.kobj, "%s", stm_data->name);
> + if (err)
> + goto err_device;
> +
> + err = device_add(&stm->dev);
> + if (err)
> + goto err_device;
> +
> + spin_lock_init(&stm->link_lock);
> + INIT_LIST_HEAD(&stm->link_list);
> +
> + spin_lock_init(&stm->mc_lock);
> + mutex_init(&stm->policy_mutex);
> + stm->sw_nmasters = nmasters;
> + stm->owner = owner;
> + stm->data = stm_data;
> + stm_data->stm = stm;
> +
> + return 0;
> +
> +err_device:
> + put_device(&stm->dev);
> +err_free:
> + kfree(stm);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(stm_register_device);
> +
> +static void __stm_source_link_drop(struct stm_source_device *src,
> + struct stm_device *stm);
> +
> +void stm_unregister_device(struct stm_data *stm_data)
> +{
> + struct stm_device *stm = stm_data->stm;
> + struct stm_source_device *src, *iter;
> + int i;
> +
> + spin_lock(&stm->link_lock);
> + list_for_each_entry_safe(src, iter, &stm->link_list, link_entry) {
> + __stm_source_link_drop(src, stm);
> + }
> + spin_unlock(&stm->link_lock);
> +
> + synchronize_srcu(&stm_source_srcu);
> +
> + unregister_chrdev(stm->major, stm_data->name);
> +
> + mutex_lock(&stm->policy_mutex);
> + if (stm->policy)
> + stp_policy_unbind(stm->policy);
> + mutex_unlock(&stm->policy_mutex);
> +
> + for (i = 0; i < stm->sw_nmasters; i++)
> + stp_master_free(stm, i);
> +
> + device_unregister(&stm->dev);
> + stm_data->stm = NULL;
> +}
> +EXPORT_SYMBOL_GPL(stm_unregister_device);
> +
> +/**
> + * stm_source_link_add() - connect an stm_source device to an stm device
> + * @src: stm_source device
> + * @stm: stm device
> + *
> + * This function establishes a link from stm_source to an stm device so that
> + * the former can send out trace data to the latter.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int stm_source_link_add(struct stm_source_device *src,
> + struct stm_device *stm)
> +{
> + char *id;
> + int err;
> +
> + spin_lock(&stm->link_lock);
> + spin_lock(&src->link_lock);
> +
> + /* src->link is dereferenced under stm_source_srcu but not the list */
> + rcu_assign_pointer(src->link, stm);
> + list_add_tail(&src->link_entry, &stm->link_list);
> +
> + spin_unlock(&src->link_lock);
> + spin_unlock(&stm->link_lock);
> +
> + id = kstrdup(src->data->name, GFP_KERNEL);
> + if (id) {
> + src->policy_node =
> + stp_policy_node_lookup(stm, id);
> +
> + kfree(id);
> + }
> +
> + err = stm_output_assign(stm, src->data->nr_chans,
> + src->policy_node, &src->output);
> +
> + if (src->policy_node)
> + stp_policy_node_put(src->policy_node);
> +
> + if (err)
> + goto fail_detach;
> +
> + /* this is to notify the STM device that a new link has been made */
> + if (stm->data->link)
> + err = stm->data->link(stm->data, src->output.master,
> + src->output.channel);
> +
> + if (err)
> + goto fail_free_output;
> +
> + /* this is to let the source carry out all necessary preparations */
> + if (src->data->link)
> + src->data->link(src->data);
> +
> + return 0;
> +
> +fail_free_output:
> + stm_output_free(stm, &src->output);
> + stm_put_device(stm);
> +
> +fail_detach:
> + spin_lock(&stm->link_lock);
> + spin_lock(&src->link_lock);
> +
> + rcu_assign_pointer(src->link, NULL);
> + list_del_init(&src->link_entry);
> +
> + spin_unlock(&src->link_lock);
> + spin_unlock(&stm->link_lock);
> +
> + return err;
> +}
> +
> +/**
> + * __stm_source_link_drop() - detach stm_source from an stm device
> + * @src: stm_source device
> + * @stm: stm device
> + *
> + * If @stm is @src::link, disconnect them from one another and put the
> + * reference on the @stm device.
> + *
> + * Caller must hold stm::link_lock.
> + */
> +static void __stm_source_link_drop(struct stm_source_device *src,
> + struct stm_device *stm)
> +{
> + spin_lock(&src->link_lock);
> + if (WARN_ON_ONCE(src->link != stm)) {
> + spin_unlock(&src->link_lock);
> + return;
> + }
> +
> + stm_output_free(src->link, &src->output);
> + /* caller must hold stm::link_lock */
> + list_del_init(&src->link_entry);
> + /* matches stm_find_device() from stm_source_link_store() */
> + stm_put_device(src->link);
> + rcu_assign_pointer(src->link, NULL);
> +
> + spin_unlock(&src->link_lock);
> +}
> +
> +/**
> + * stm_source_link_drop() - detach stm_source from its stm device
> + * @src: stm_source device
> + *
> + * Unlinking means disconnecting from source's STM device; after this
> + * writes will be unsuccessful until it is linked to a new STM device.
> + *
> + * This will happen on "stm_source_link" sysfs attribute write to undo
> + * the existing link (if any), or on linked STM device's de-registration.
> + */
> +static void stm_source_link_drop(struct stm_source_device *src)
> +{
> + struct stm_device *stm;
> + int idx;
> +
> + idx = srcu_read_lock(&stm_source_srcu);
> + stm = srcu_dereference(src->link, &stm_source_srcu);
> +
> + if (stm) {
> + if (src->data->unlink)
> + src->data->unlink(src->data);
> +
> + spin_lock(&stm->link_lock);
> + __stm_source_link_drop(src, stm);
> + spin_unlock(&stm->link_lock);
> + }
> +
> + srcu_read_unlock(&stm_source_srcu, idx);
> +}
> +
> +static ssize_t stm_source_link_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct stm_source_device *src = to_stm_source_device(dev);
> + struct stm_device *stm;
> + int idx, ret;
> +
> + idx = srcu_read_lock(&stm_source_srcu);
> + stm = srcu_dereference(src->link, &stm_source_srcu);
> + ret = sprintf(buf, "%s\n",
> + stm ? dev_name(&stm->dev) : "<none>");
> + srcu_read_unlock(&stm_source_srcu, idx);
> +
> + return ret;
> +}
> +
> +static ssize_t stm_source_link_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct stm_source_device *src = to_stm_source_device(dev);
> + struct stm_device *link;
> + int err;
> +
> + stm_source_link_drop(src);
> +
> + link = stm_find_device(buf);
> + if (!link)
> + return -EINVAL;
> +
> + err = stm_source_link_add(src, link);
> + if (err)
> + stm_put_device(link);
> +
> + return err ? : count;
> +}
> +
> +static DEVICE_ATTR_RW(stm_source_link);
> +
> +static struct attribute *stm_source_attrs[] = {
> + &dev_attr_stm_source_link.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(stm_source);
> +
> +static struct class stm_source_class = {
> + .name = "stm_source",
> + .dev_groups = stm_source_groups,
> +};
> +
> +static void stm_source_device_release(struct device *dev)
> +{
> + struct stm_source_device *src = to_stm_source_device(dev);
> +
> + kfree(src);
> +}
> +
> +/**
> + * stm_source_register_device() - register an stm_source device
> + * @parent: parent device
> + * @data: device description structure
> + *
> + * This will create a device of stm_source class that can write
> + * data to an stm device once linked.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +int stm_source_register_device(struct device *parent,
> + struct stm_source_data *data)
> +{
> + struct stm_source_device *src;
> + int err;
> +
> + if (!stm_core_up)
> + return -EPROBE_DEFER;
> +
I tried to update Coresight-stm driver[1] based on your this version
patch, but the Coresight-stm driver probe() failed.
the reason was:
In the end of Coresight stm_probe(), we called this function, but
"stm_core_up" was zero then, so the error returned value
"-EPROBE_DEFER" was received.
In fact, "stm_core_up" would increase itself until "stm_core_init" be
called - it's the root of this problem, I'll explain this where the
function "stm_core_init" defined.
And redoing Coresight stm_probe() will incur a WARN_ON() like below:
[ 1.075746] coresight-stm 10006000.stm: stm_register_device failed
[ 1.082118] ------------[ cut here ]------------
[ 1.086819] WARNING: CPU: 1 PID: 1 at drivers/clk/clk.c:657
clk_core_disable+0x138/0x13c()
[ 1.095353] Modules linked in:
[ 1.098487] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G S
4.2.0-rc1+ #107
[ 1.106398] Hardware name: Spreadtrum SC9836 Openphone Board (DT)
[ 1.112678] Call trace:
[ 1.115194] [<ffffffc00008a5b4>] dump_backtrace+0x0/0x138
[ 1.120761] [<ffffffc00008a708>] show_stack+0x1c/0x28
[ 1.125972] [<ffffffc0003320e0>] dump_stack+0x84/0xc8
[ 1.131179] [<ffffffc00009b580>] warn_slowpath_common+0xa4/0xdc
[ 1.137285] [<ffffffc00009b700>] warn_slowpath_null+0x34/0x44
[ 1.143213] [<ffffffc000321eb4>] clk_core_disable+0x134/0x13c
[ 1.149140] [<ffffffc000322154>] clk_disable+0x30/0x48
[ 1.154439] [<ffffffc0002b5e58>] amba_put_disable_pclk.isra.6+0x20/0x40
[ 1.161264] [<ffffffc0002b6134>] amba_probe+0x16c/0x19c
[ 1.166653] [<ffffffc0002e664c>] driver_probe_device+0x1a4/0x2b4
[ 1.172849] [<ffffffc0002e6804>] __driver_attach+0xa8/0xb0
[ 1.178506] [<ffffffc0002e45fc>] bus_for_each_dev+0x68/0xac
[ 1.184253] [<ffffffc0002e60b4>] driver_attach+0x28/0x34
[ 1.189731] [<ffffffc0002e5cd0>] bus_add_driver+0x1cc/0x234
[ 1.195479] [<ffffffc0002e73d8>] driver_register+0x64/0x124
[ 1.201226] [<ffffffc0002b58cc>] amba_driver_register+0x64/0x74
[ 1.207333] [<ffffffc00045591c>] stm_driver_init+0x10/0x1c
[ 1.212990] [<ffffffc0000828e0>] do_one_initcall+0x90/0x1a8
[ 1.218739] [<ffffffc000431adc>] kernel_init_freeable+0x150/0x1f4
[ 1.225026] [<ffffffc00032fbe4>] kernel_init+0x1c/0xe0
[ 1.230321] ---[ end trace 487f4b7838ebf86a ]---
[ 1.235102] ------------[ cut here ]------------
> + src = kzalloc(sizeof(*src), GFP_KERNEL);
> + if (!src)
> + return -ENOMEM;
> +
> + device_initialize(&src->dev);
> + src->dev.class = &stm_source_class;
> + src->dev.parent = parent;
> + src->dev.release = stm_source_device_release;
> +
> + err = kobject_set_name(&src->dev.kobj, "%s", data->name);
> + if (err)
> + goto err;
> +
> + err = device_add(&src->dev);
> + if (err)
> + goto err;
> +
> + spin_lock_init(&src->link_lock);
> + INIT_LIST_HEAD(&src->link_entry);
> + src->data = data;
> + data->src = src;
> +
> + return 0;
> +
> +err:
> + put_device(&src->dev);
> + kfree(src);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(stm_source_register_device);
> +
> +/**
> + * stm_source_unregister_device() - unregister an stm_source device
> + * @data: device description that was used to register the device
> + *
> + * This will remove a previously created stm_source device from the system.
> + */
> +void stm_source_unregister_device(struct stm_source_data *data)
> +{
> + struct stm_source_device *src = data->src;
> +
> + stm_source_link_drop(src);
> +
> + device_destroy(&stm_source_class, src->dev.devt);
> +}
> +EXPORT_SYMBOL_GPL(stm_source_unregister_device);
> +
> +int stm_source_write(struct stm_source_data *data, unsigned int chan,
> + const char *buf, size_t count)
> +{
> + struct stm_source_device *src = data->src;
> + struct stm_device *stm;
> + int idx;
> +
> + if (!src->output.nr_chans)
> + return -ENODEV;
> +
> + if (chan >= src->output.nr_chans)
> + return -EINVAL;
> +
> + idx = srcu_read_lock(&stm_source_srcu);
> +
> + stm = srcu_dereference(src->link, &stm_source_srcu);
> + if (stm)
> + stm_write(stm->data, src->output.master,
> + src->output.channel + chan,
> + buf, count);
> + else
> + count = -ENODEV;
> +
> + srcu_read_unlock(&stm_source_srcu, idx);
> +
> + return count;
> +}
> +EXPORT_SYMBOL_GPL(stm_source_write);
> +
> +static int __init stm_core_init(void)
> +{
> + int err;
> +
> + err = class_register(&stm_class);
> + if (err)
> + return err;
> +
> + err = class_register(&stm_source_class);
> + if (err)
> + goto err_stm;
> +
> + err = stp_configfs_init();
> + if (err)
> + goto err_src;
> +
> + init_srcu_struct(&stm_source_srcu);
> +
> + stm_core_up++;
> +
> + return 0;
> +
> +err_src:
> + class_unregister(&stm_source_class);
> +err_stm:
> + class_unregister(&stm_class);
> +
> + return err;
> +}
> +
> +module_init(stm_core_init);
Since you are using module_init() instead of postcore_initcall() which
was in the last version patch, as such, this function would be
executed after Coresight "stm_probe" finished.
So, we think there a few optional solutions:
1) Remove the "stm_register_device" out from Coresight "stm_probe",
but we have to save another global variable:
struct device *stm_dev;
in the process of Coresight "stm_probe".
2) Change module_init() to other XYX_init() which would run prior to
"amba_probe()" (i.e. the caller of Coresight stm_probe), this may be a
better one.
3) stm_core_init() could be turned into a library call where
initialisation of the internals is done when first called.
Looking forward to your reply/comments.
Thanks,
Chunyan
[1] https://lkml.org/lkml/2015/2/4/729
> +
> +static void __exit stm_core_exit(void)
> +{
> + cleanup_srcu_struct(&stm_source_srcu);
> + class_unregister(&stm_source_class);
> + class_unregister(&stm_class);
> + stp_configfs_exit();
> +}
> +
> +module_exit(stm_core_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("System Trace Module device class");
> +MODULE_AUTHOR("Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>");
> diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c
> new file mode 100644
> index 0000000000..6498a9dbb7
> --- /dev/null
> +++ b/drivers/hwtracing/stm/policy.c
> @@ -0,0 +1,529 @@
> +/*
> + * System Trace Module (STM) master/channel allocation policy management
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * A master/channel allocation policy allows mapping string identifiers to
> + * master and channel ranges, where allocation can be done.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/configfs.h>
> +#include <linux/slab.h>
> +#include <linux/stm.h>
> +#include "stm.h"
> +
> +/*
> + * STP Master/Channel allocation policy configfs layout.
> + */
> +
> +struct stp_policy {
> + struct config_group group;
> + struct stm_device *stm;
> +};
> +
> +struct stp_policy_node {
> + struct config_group group;
> + struct stp_policy *policy;
> + unsigned int first_master;
> + unsigned int last_master;
> + unsigned int first_channel;
> + unsigned int last_channel;
> +};
> +
> +static struct configfs_subsystem stp_policy_subsys;
> +
> +void stp_policy_node_get_ranges(struct stp_policy_node *policy_node,
> + unsigned int *mstart, unsigned int *mend,
> + unsigned int *cstart, unsigned int *cend)
> +{
> + *mstart = policy_node->first_master;
> + *mend = policy_node->last_master;
> + *cstart = policy_node->first_channel;
> + *cend = policy_node->last_channel;
> +}
> +
> +static inline char *stp_policy_node_name(struct stp_policy_node *policy_node)
> +{
> + return policy_node->group.cg_item.ci_name ? : "<none>";
> +}
> +
> +static inline struct stp_policy *to_stp_policy(struct config_item *item)
> +{
> + return item ?
> + container_of(to_config_group(item), struct stp_policy, group) :
> + NULL;
> +}
> +
> +static inline struct stp_policy_node *
> +to_stp_policy_node(struct config_item *item)
> +{
> + return item ?
> + container_of(to_config_group(item), struct stp_policy_node,
> + group) :
> + NULL;
> +}
> +
> +static ssize_t stp_policy_node_masters_show(struct stp_policy_node *policy_node,
> + char *page)
> +{
> + ssize_t count;
> +
> + count = sprintf(page, "%u %u\n", policy_node->first_master,
> + policy_node->last_master);
> +
> + return count;
> +}
> +
> +static ssize_t
> +stp_policy_node_masters_store(struct stp_policy_node *policy_node,
> + const char *page, size_t count)
> +{
> + unsigned int first, last;
> + struct stm_device *stm;
> + char *p = (char *)page;
> + ssize_t ret = -ENODEV;
> +
> + if (sscanf(p, "%u %u", &first, &last) != 2)
> + return -EINVAL;
> +
> + mutex_lock(&stp_policy_subsys.su_mutex);
> + stm = policy_node->policy->stm;
> + if (!stm)
> + goto unlock;
> +
> + /* must be within [sw_start..sw_end], which is an inclusive range */
> + if (first > INT_MAX || last > INT_MAX || first > last ||
> + first < stm->data->sw_start ||
> + last > stm->data->sw_end) {
> + ret = -ERANGE;
> + goto unlock;
> + }
> +
> + ret = count;
> + policy_node->first_master = first;
> + policy_node->last_master = last;
> +
> +unlock:
> + mutex_unlock(&stp_policy_subsys.su_mutex);
> +
> + return ret;
> +}
> +
> +static ssize_t
> +stp_policy_node_channels_show(struct stp_policy_node *policy_node, char *page)
> +{
> + ssize_t count;
> +
> + count = sprintf(page, "%u %u\n", policy_node->first_channel,
> + policy_node->last_channel);
> +
> + return count;
> +}
> +
> +static ssize_t
> +stp_policy_node_channels_store(struct stp_policy_node *policy_node,
> + const char *page, size_t count)
> +{
> + unsigned int first, last;
> + struct stm_device *stm;
> + char *p = (char *)page;
> + ssize_t ret = -ENODEV;
> +
> + if (sscanf(p, "%u %u", &first, &last) != 2)
> + return -EINVAL;
> +
> + mutex_lock(&stp_policy_subsys.su_mutex);
> + stm = policy_node->policy->stm;
> + if (!stm)
> + goto unlock;
> +
> + if (first > INT_MAX || last > INT_MAX || first > last ||
> + last >= stm->data->sw_nchannels) {
> + ret = -ERANGE;
> + goto unlock;
> + }
> +
> + ret = count;
> + policy_node->first_channel = first;
> + policy_node->last_channel = last;
> +
> +unlock:
> + mutex_unlock(&stp_policy_subsys.su_mutex);
> +
> + return ret;
> +}
> +
> +static void stp_policy_node_release(struct config_item *item)
> +{
> + kfree(to_stp_policy_node(item));
> +}
> +
> +struct stp_policy_node_attribute {
> + struct configfs_attribute attr;
> + ssize_t (*show)(struct stp_policy_node *, char *);
> + ssize_t (*store)(struct stp_policy_node *, const char *, size_t);
> +};
> +
> +static ssize_t stp_policy_node_attr_show(struct config_item *item,
> + struct configfs_attribute *attr,
> + char *page)
> +{
> + struct stp_policy_node *policy_node = to_stp_policy_node(item);
> + struct stp_policy_node_attribute *pn_attr =
> + container_of(attr, struct stp_policy_node_attribute, attr);
> + ssize_t count = 0;
> +
> + if (pn_attr->show)
> + count = pn_attr->show(policy_node, page);
> +
> + return count;
> +}
> +
> +static ssize_t stp_policy_node_attr_store(struct config_item *item,
> + struct configfs_attribute *attr,
> + const char *page, size_t len)
> +{
> + struct stp_policy_node *policy_node = to_stp_policy_node(item);
> + struct stp_policy_node_attribute *pn_attr =
> + container_of(attr, struct stp_policy_node_attribute, attr);
> + ssize_t count = -EINVAL;
> +
> + if (pn_attr->store)
> + count = pn_attr->store(policy_node, page, len);
> +
> + return count;
> +}
> +
> +static struct configfs_item_operations stp_policy_node_item_ops = {
> + .release = stp_policy_node_release,
> + .show_attribute = stp_policy_node_attr_show,
> + .store_attribute = stp_policy_node_attr_store,
> +};
> +
> +static struct stp_policy_node_attribute stp_policy_node_attr_range = {
> + .attr = {
> + .ca_owner = THIS_MODULE,
> + .ca_name = "masters",
> + .ca_mode = S_IRUGO | S_IWUSR,
> + },
> + .show = stp_policy_node_masters_show,
> + .store = stp_policy_node_masters_store,
> +};
> +
> +static struct stp_policy_node_attribute stp_policy_node_attr_channels = {
> + .attr = {
> + .ca_owner = THIS_MODULE,
> + .ca_name = "channels",
> + .ca_mode = S_IRUGO | S_IWUSR,
> + },
> + .show = stp_policy_node_channels_show,
> + .store = stp_policy_node_channels_store,
> +};
> +
> +static struct configfs_attribute *stp_policy_node_attrs[] = {
> + &stp_policy_node_attr_range.attr,
> + &stp_policy_node_attr_channels.attr,
> + NULL,
> +};
> +
> +static struct config_item_type stp_policy_type;
> +static struct config_item_type stp_policy_node_type;
> +
> +static struct config_group *
> +stp_policy_node_make(struct config_group *group, const char *name)
> +{
> + struct stp_policy_node *policy_node, *parent_node;
> + struct stp_policy *policy;
> +
> + if (group->cg_item.ci_type == &stp_policy_type) {
> + policy = container_of(group, struct stp_policy, group);
> + } else {
> + parent_node = container_of(group, struct stp_policy_node,
> + group);
> + policy = parent_node->policy;
> + }
> +
> + if (!policy->stm)
> + return ERR_PTR(-ENODEV);
> +
> + policy_node = kzalloc(sizeof(struct stp_policy_node), GFP_KERNEL);
> + if (!policy_node)
> + return ERR_PTR(-ENOMEM);
> +
> + config_group_init_type_name(&policy_node->group, name,
> + &stp_policy_node_type);
> +
> + policy_node->policy = policy;
> +
> + /* default values for the attributes */
> + policy_node->first_master = policy->stm->data->sw_start;
> + policy_node->last_master = policy->stm->data->sw_end;
> + policy_node->first_channel = 0;
> + policy_node->last_channel = policy->stm->data->sw_nchannels - 1;
> +
> + return &policy_node->group;
> +}
> +
> +static void
> +stp_policy_node_drop(struct config_group *group, struct config_item *item)
> +{
> + config_item_put(item);
> +}
> +
> +static struct configfs_group_operations stp_policy_node_group_ops = {
> + .make_group = stp_policy_node_make,
> + .drop_item = stp_policy_node_drop,
> +};
> +
> +static struct config_item_type stp_policy_node_type = {
> + .ct_item_ops = &stp_policy_node_item_ops,
> + .ct_group_ops = &stp_policy_node_group_ops,
> + .ct_attrs = stp_policy_node_attrs,
> + .ct_owner = THIS_MODULE,
> +};
> +
> +/*
> + * Root group: policies.
> + */
> +static struct configfs_attribute stp_policy_attr_device = {
> + .ca_owner = THIS_MODULE,
> + .ca_name = "device",
> + .ca_mode = S_IRUGO,
> +};
> +
> +static struct configfs_attribute *stp_policy_attrs[] = {
> + &stp_policy_attr_device,
> + NULL,
> +};
> +
> +static ssize_t stp_policy_attr_show(struct config_item *item,
> + struct configfs_attribute *attr,
> + char *page)
> +{
> + struct stp_policy *policy = to_stp_policy(item);
> + ssize_t count;
> +
> + count = sprintf(page, "%s\n",
> + (policy && policy->stm) ?
> + policy->stm->data->name :
> + "<none>");
> +
> + return count;
> +}
> +
> +void stp_policy_unbind(struct stp_policy *policy)
> +{
> + struct stm_device *stm = policy->stm;
> +
> + if (WARN_ON_ONCE(!policy->stm))
> + return;
> +
> + mutex_lock(&stm->policy_mutex);
> + stm->policy = NULL;
> + mutex_unlock(&stm->policy_mutex);
> +
> + policy->stm = NULL;
> +
> + stm_put_device(stm);
> +}
> +
> +static void stp_policy_release(struct config_item *item)
> +{
> + struct stp_policy *policy = to_stp_policy(item);
> +
> + stp_policy_unbind(policy);
> + kfree(policy);
> +}
> +
> +static struct configfs_item_operations stp_policy_item_ops = {
> + .release = stp_policy_release,
> + .show_attribute = stp_policy_attr_show,
> +};
> +
> +static struct configfs_group_operations stp_policy_group_ops = {
> + .make_group = stp_policy_node_make,
> +};
> +
> +static struct config_item_type stp_policy_type = {
> + .ct_item_ops = &stp_policy_item_ops,
> + .ct_group_ops = &stp_policy_group_ops,
> + .ct_attrs = stp_policy_attrs,
> + .ct_owner = THIS_MODULE,
> +};
> +
> +static struct config_group *
> +stp_policies_make(struct config_group *group, const char *name)
> +{
> + struct config_group *ret;
> + struct stm_device *stm;
> + char *devname, *p;
> +
> + devname = kasprintf(GFP_KERNEL, "%s", name);
> + if (!devname)
> + return ERR_PTR(-ENOMEM);
> +
> + /*
> + * node must look like <device_name>.<policy_name>, where
> + * <device_name> is the name of an existing stm device and
> + * <policy_name> is an arbitrary string
> + */
> + p = strchr(devname, '.');
> + if (!p) {
> + kfree(devname);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + *p++ = '\0';
> +
> + stm = stm_find_device(devname);
> + kfree(devname);
> +
> + if (!stm)
> + return ERR_PTR(-ENODEV);
> +
> + mutex_lock(&stm->policy_mutex);
> + if (stm->policy) {
> + ret = ERR_PTR(-EBUSY);
> + goto unlock_policy;
> + }
> +
> + stm->policy = kzalloc(sizeof(*stm->policy), GFP_KERNEL);
> + if (!stm->policy) {
> + ret = ERR_PTR(-ENOMEM);
> + goto unlock_policy;
> + }
> +
> + config_group_init_type_name(&stm->policy->group, name,
> + &stp_policy_type);
> + stm->policy->stm = stm;
> +
> + ret = &stm->policy->group;
> +
> +unlock_policy:
> + mutex_unlock(&stm->policy_mutex);
> +
> + if (IS_ERR(ret))
> + stm_put_device(stm);
> +
> + return ret;
> +}
> +
> +static struct configfs_group_operations stp_policies_group_ops = {
> + .make_group = stp_policies_make,
> +};
> +
> +static struct config_item_type stp_policies_type = {
> + .ct_group_ops = &stp_policies_group_ops,
> + .ct_owner = THIS_MODULE,
> +};
> +
> +static struct configfs_subsystem stp_policy_subsys = {
> + .su_group = {
> + .cg_item = {
> + .ci_namebuf = "stp-policy",
> + .ci_type = &stp_policies_type,
> + },
> + },
> +};
> +
> +/*
> + * Lock the policy mutex from the outside
> + */
> +static struct stp_policy_node *
> +__stp_policy_node_lookup(struct stp_policy *policy, char *s)
> +{
> + struct stp_policy_node *policy_node, *ret;
> + struct list_head *head = &policy->group.cg_children;
> + struct config_item *item;
> + char *start, *end = s;
> +
> + if (list_empty(head))
> + return NULL;
> +
> + /* return the first entry if everything else fails */
> + item = list_entry(head->next, struct config_item, ci_entry);
> + ret = to_stp_policy_node(item);
> +
> +next:
> + for (;;) {
> + start = strsep(&end, "/");
> + if (!start)
> + break;
> +
> + if (!*start)
> + continue;
> +
> + list_for_each_entry(item, head, ci_entry) {
> + policy_node = to_stp_policy_node(item);
> +
> + if (!strcmp(start,
> + policy_node->group.cg_item.ci_name)) {
> + ret = policy_node;
> +
> + if (!end)
> + goto out;
> +
> + head = &policy_node->group.cg_children;
> + goto next;
> + }
> + }
> + break;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +
> +struct stp_policy_node *
> +stp_policy_node_lookup(struct stm_device *stm, char *s)
> +{
> + struct stp_policy_node *policy_node = NULL;
> +
> + mutex_lock(&stp_policy_subsys.su_mutex);
> +
> + mutex_lock(&stm->policy_mutex);
> + if (stm->policy)
> + policy_node = __stp_policy_node_lookup(stm->policy, s);
> + mutex_unlock(&stm->policy_mutex);
> +
> + if (policy_node)
> + config_item_get(&policy_node->group.cg_item);
> + mutex_unlock(&stp_policy_subsys.su_mutex);
> +
> + return policy_node;
> +}
> +
> +void stp_policy_node_put(struct stp_policy_node *policy_node)
> +{
> + config_item_put(&policy_node->group.cg_item);
> +}
> +
> +int __init stp_configfs_init(void)
> +{
> + int err;
> +
> + config_group_init(&stp_policy_subsys.su_group);
> + mutex_init(&stp_policy_subsys.su_mutex);
> + err = configfs_register_subsystem(&stp_policy_subsys);
> +
> + return err;
> +}
> +
> +void __exit stp_configfs_exit(void)
> +{
> + configfs_unregister_subsystem(&stp_policy_subsys);
> +}
> diff --git a/drivers/hwtracing/stm/stm.h b/drivers/hwtracing/stm/stm.h
> new file mode 100644
> index 0000000000..cf33bf976a
> --- /dev/null
> +++ b/drivers/hwtracing/stm/stm.h
> @@ -0,0 +1,87 @@
> +/*
> + * System Trace Module (STM) infrastructure
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * STM class implements generic infrastructure for System Trace Module devices
> + * as defined in MIPI STPv2 specification.
> + */
> +
> +#ifndef _STM_STM_H_
> +#define _STM_STM_H_
> +
> +struct stp_policy;
> +struct stp_policy_node;
> +
> +struct stp_policy_node *
> +stp_policy_node_lookup(struct stm_device *stm, char *s);
> +void stp_policy_node_put(struct stp_policy_node *policy_node);
> +void stp_policy_unbind(struct stp_policy *policy);
> +
> +void stp_policy_node_get_ranges(struct stp_policy_node *policy_node,
> + unsigned int *mstart, unsigned int *mend,
> + unsigned int *cstart, unsigned int *cend);
> +int stp_configfs_init(void);
> +void stp_configfs_exit(void);
> +
> +struct stp_master {
> + unsigned int nr_free;
> + unsigned long chan_map[0];
> +};
> +
> +struct stm_device {
> + struct device dev;
> + struct module *owner;
> + struct stp_policy *policy;
> + struct mutex policy_mutex;
> + int major;
> + unsigned int sw_nmasters;
> + struct stm_data *data;
> + spinlock_t link_lock;
> + struct list_head link_list;
> + /* master allocation */
> + spinlock_t mc_lock;
> + struct stp_master *masters[0];
> +};
> +
> +#define to_stm_device(_d) \
> + container_of((_d), struct stm_device, dev)
> +
> +struct stm_output {
> + unsigned int master;
> + unsigned int channel;
> + unsigned int nr_chans;
> +};
> +
> +struct stm_file {
> + struct stm_device *stm;
> + struct stp_policy_node *policy_node;
> + struct stm_output output;
> +};
> +
> +struct stm_device *stm_find_device(const char *name);
> +void stm_put_device(struct stm_device *stm);
> +
> +struct stm_source_device {
> + struct device dev;
> + struct stm_source_data *data;
> + spinlock_t link_lock;
> + struct stm_device *link;
> + struct list_head link_entry;
> + /* one output per stm_source device */
> + struct stp_policy_node *policy_node;
> + struct stm_output output;
> +};
> +
> +#define to_stm_source_device(_d) \
> + container_of((_d), struct stm_source_device, dev)
> +
> +#endif /* _STM_STM_H_ */
> diff --git a/include/linux/stm.h b/include/linux/stm.h
> new file mode 100644
> index 0000000000..9d0083d364
> --- /dev/null
> +++ b/include/linux/stm.h
> @@ -0,0 +1,126 @@
> +/*
> + * System Trace Module (STM) infrastructure apis
> + * Copyright (C) 2014 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#ifndef _STM_H_
> +#define _STM_H_
> +
> +#include <linux/device.h>
> +
> +/**
> + * enum stp_packet_type - STP packets that an STM driver sends
> + */
> +enum stp_packet_type {
> + STP_PACKET_DATA = 0,
> + STP_PACKET_FLAG,
> + STP_PACKET_USER,
> + STP_PACKET_MERR,
> + STP_PACKET_GERR,
> + STP_PACKET_TRIG,
> + STP_PACKET_XSYNC,
> +};
> +
> +/**
> + * enum stp_packet_flags - STP packet modifiers
> + */
> +enum stp_packet_flags {
> + STP_PACKET_MARKED = 0x1,
> + STP_PACKET_TIMESTAMPED = 0x2,
> +};
> +
> +struct stp_policy;
> +
> +struct stm_device;
> +
> +/**
> + * struct stm_data - STM device description and callbacks
> + * @name: device name
> + * @stm: internal structure, only used by stm class code
> + * @sw_start: first STP master available to software
> + * @sw_end: last STP master available to software
> + * @sw_nchannels: number of STP channels per master
> + * @sw_mmiosz: size of one channel's IO space, for mmap, optional
> + * @packet: callback that sends an STP packet
> + * @mmio_addr: mmap callback, optional
> + * @link: called when a new stm_source gets linked to us, optional
> + * @unlink: likewise for unlinking, again optional
> + * @set_options: set device-specific options on a channel
> + *
> + * Fill out this structure before calling stm_register_device() to create
> + * an STM device and stm_unregister_device() to destroy it. It will also be
> + * passed back to @packet(), @mmio_addr(), @link(), @unlink() and @set_options()
> + * callbacks.
> + *
> + * Normally, an STM device will have a range of masters available to software
> + * and the rest being statically assigned to various hardware trace sources.
> + * The former is defined by the the range [@sw_start..@sw_end] of the device
> + * description. That is, the lowest master that can be allocated to software
> + * writers is @sw_start and data from this writer will appear is @sw_start
> + * master in the STP stream.
> + */
> +struct stm_data {
> + const char *name;
> + struct stm_device *stm;
> + unsigned int sw_start;
> + unsigned int sw_end;
> + unsigned int sw_nchannels;
> + unsigned int sw_mmiosz;
> + ssize_t (*packet)(struct stm_data *, unsigned int,
> + unsigned int, unsigned int,
> + unsigned int, unsigned int,
> + const unsigned char *);
> + phys_addr_t (*mmio_addr)(struct stm_data *, unsigned int,
> + unsigned int, unsigned int);
> + int (*link)(struct stm_data *, unsigned int,
> + unsigned int);
> + void (*unlink)(struct stm_data *, unsigned int,
> + unsigned int);
> + long (*set_options)(struct stm_data *, unsigned int,
> + unsigned int, unsigned int,
> + unsigned long);
> +};
> +
> +int stm_register_device(struct device *parent, struct stm_data *stm_data,
> + struct module *owner);
> +void stm_unregister_device(struct stm_data *stm_data);
> +
> +struct stm_source_device;
> +
> +/**
> + * struct stm_source_data - STM source device description and callbacks
> + * @name: device name, will be used for policy lookup
> + * @src: internal structure, only used by stm class code
> + * @nr_chans: number of channels to allocate
> + * @link: called when this source gets linked to an STM device
> + * @unlink: called when this source is about to get unlinked from its STM
> + *
> + * Fill in this structure before calling stm_source_register_device() to
> + * register a source device. Also pass it to unregister and write calls.
> + */
> +struct stm_source_data {
> + const char *name;
> + struct stm_source_device *src;
> + unsigned int percpu;
> + unsigned int nr_chans;
> + int (*link)(struct stm_source_data *data);
> + void (*unlink)(struct stm_source_data *data);
> +};
> +
> +int stm_source_register_device(struct device *parent,
> + struct stm_source_data *data);
> +void stm_source_unregister_device(struct stm_source_data *data);
> +
> +int stm_source_write(struct stm_source_data *data, unsigned int chan,
> + const char *buf, size_t count);
> +
> +#endif /* _STM_H_ */
> diff --git a/include/uapi/linux/stm.h b/include/uapi/linux/stm.h
> new file mode 100644
> index 0000000000..626a8d3f63
> --- /dev/null
> +++ b/include/uapi/linux/stm.h
> @@ -0,0 +1,50 @@
> +/*
> + * System Trace Module (STM) userspace interfaces
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * STM class implements generic infrastructure for System Trace Module devices
> + * as defined in MIPI STPv2 specification.
> + */
> +
> +#ifndef _UAPI_LINUX_STM_H
> +#define _UAPI_LINUX_STM_H
> +
> +#include <linux/types.h>
> +
> +/**
> + * struct stp_policy_id - identification for the STP policy
> + * @size: size of the structure including real id[] length
> + * @master: assigned master
> + * @channel: first assigned channel
> + * @width: number of requested channels
> + * @id: identification string
> + *
> + * User must calculate the total size of the structure and put it into
> + * @size field, fill out the @id and desired @width. In return, kernel
> + * fills out @master, @channel and @width.
> + */
> +struct stp_policy_id {
> + __u32 size;
> + __u16 master;
> + __u16 channel;
> + __u16 width;
> + /* padding */
> + __u16 __reserved_0;
> + __u32 __reserved_1;
> + char id[0];
> +};
> +
> +#define STP_POLICY_ID_SET _IOWR('%', 0, struct stp_policy_id)
> +#define STP_POLICY_ID_GET _IOR('%', 1, struct stp_policy_id)
> +#define STP_SET_OPTIONS _IOW('%', 2, __u64)
> +
> +#endif /* _UAPI_LINUX_STM_H */
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Darren Hart @ 2015-07-29 4:21 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Michael Kerrisk (man-pages), Torvald Riegel, Carlos O'Donell,
Ingo Molnar, Jakub Jelinek, linux-man, lkml, Davidlohr Bueso,
Arnd Bergmann, Steven Rostedt, Peter Zijlstra, Linux API,
Roland McGrath, Anton Blanchard, Eric Dumazet, bill o gallmeister,
Jan Kiszka, Daniel Wagner, Rich Felker, Andy Lutomirski,
bert hubert, Rusty Russell, Heinrich Schuchardt
In-Reply-To: <20150729041141.GE3171@vmdeb7>
On Tue, Jul 28, 2015 at 09:11:41PM -0700, Darren Hart wrote:
> On Tue, Jul 28, 2015 at 10:23:51PM +0200, Thomas Gleixner wrote:
> > On Mon, 27 Jul 2015, Michael Kerrisk (man-pages) wrote:
>
> ...
>
> > > FUTEX_REQUEUE (since Linux 2.6.0)
> > > .\" FIXME(Torvald) Is there some indication that FUTEX_REQUEUE is broken
> > > .\" in general, or is this comment implicitly speaking about the
> > > .\" condvar (?) use case? If the latter we might want to weaken the
> > > .\" advice below a little.
> > > .\" [Anyone else have input on this?]
> >
> > The condvar use case exposes the flaw nicely, but that's pretty much
> > true for everything which wants a sane requeue operation.
>
> In an earlier discussion I argued this point (that FUTURE_REQUEUE is broken and
> should not be used in new code) and someone argued strongly against... stating
> that there were legitimate uses for it. Of course I'm struggling to find the
> thread and the reference at the moment - immensely useful, I know.
>
> I'll continue trying to find it and see if it can be useful here. I believe
> Torvald was on the thread as well.
>
Found it on libc-alpha, here it is for reference:
From: Rich Felker <dalias-8zAoT0mYgF4@public.gmane.org>
Date: Wed, 29 Oct 2014 22:43:17 -0400
To: Darren Hart <dvhart-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: Carlos O'Donell <carlos-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, Roland McGrath <roland-/Z5OmTQCD9xF6kxbq+BtvQ@public.gmane.org>,
Torvald Riegel <triegel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, GLIBC Devel <libc-alpha-9JcytcrH/bA+uJoB2kUjGw@public.gmane.org>,
Michael Kerrisk <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: Re: Add futex wrapper to glibc?
On Wed, Oct 29, 2014 at 06:59:15PM -0700, Darren Hart wrote:
> > We are IMO at the stage where futex is stable, few things are
> > changing, and with documentation in place, I would consider adding a
> > futex wrapper.
>
> Yes, at least for the defined OP codes. New OPs may be added of
> course, but that isn't a concern for supporting what exists today, and
> doesn't break compatibility.
>
> I wonder though... can we not wrap FUTEX_REQUEUE? It's fundamentally
> broken. FUTEX_CMP_REQUEUE should *always* be used instead. The glibc
> wrapper is one way to encourage developers to do the right thing
> (don't expose the bad op in the header).
You're mistaken here. There are plenty of valid ways to use
FUTEX_REQUEUE - for example if the calling thread is requeuing the
target(s) to a lock that the calling thread owns. Just because it
doesn't meet the needs of the way glibc was using it internally
doesn't mean it's useless for other applications.
In any case, I don't think there's a proposal to intercept/modify the
commands to futex, just to pass them through (and possibly do a
cancellable syscall for some of them).
Rich
> >
> > > Avoid using this operation. It is broken for its intended
> > > purpose. Use FUTEX_CMP_REQUEUE instead.
> > >
> > > This operation performs the same task as
> > > FUTEX_CMP_REQUEUE, except that no check is made using the
> > > value in val3. (The argument val3 is ignored.)
> > >
>
> --
> Darren Hart
> Intel Open Source Technology Center
--
Darren Hart
Intel Open Source Technology Center
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Darren Hart @ 2015-07-29 4:11 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Michael Kerrisk (man-pages), Torvald Riegel, Carlos O'Donell,
Ingo Molnar, Jakub Jelinek, linux-man, lkml, Davidlohr Bueso,
Arnd Bergmann, Steven Rostedt, Peter Zijlstra, Linux API,
Roland McGrath, Anton Blanchard, Eric Dumazet, bill o gallmeister,
Jan Kiszka, Daniel Wagner, Rich Felker, Andy Lutomirski,
bert hubert, Rusty Russell, Heinrich Schuchardt
In-Reply-To: <alpine.DEB.2.11.1507282054370.3825@nanos>
On Tue, Jul 28, 2015 at 10:23:51PM +0200, Thomas Gleixner wrote:
> On Mon, 27 Jul 2015, Michael Kerrisk (man-pages) wrote:
...
> > FUTEX_REQUEUE (since Linux 2.6.0)
> > .\" FIXME(Torvald) Is there some indication that FUTEX_REQUEUE is broken
> > .\" in general, or is this comment implicitly speaking about the
> > .\" condvar (?) use case? If the latter we might want to weaken the
> > .\" advice below a little.
> > .\" [Anyone else have input on this?]
>
> The condvar use case exposes the flaw nicely, but that's pretty much
> true for everything which wants a sane requeue operation.
In an earlier discussion I argued this point (that FUTURE_REQUEUE is broken and
should not be used in new code) and someone argued strongly against... stating
that there were legitimate uses for it. Of course I'm struggling to find the
thread and the reference at the moment - immensely useful, I know.
I'll continue trying to find it and see if it can be useful here. I believe
Torvald was on the thread as well.
>
> > Avoid using this operation. It is broken for its intended
> > purpose. Use FUTEX_CMP_REQUEUE instead.
> >
> > This operation performs the same task as
> > FUTEX_CMP_REQUEUE, except that no check is made using the
> > value in val3. (The argument val3 is ignored.)
> >
--
Darren Hart
Intel Open Source Technology Center
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Davidlohr Bueso @ 2015-07-29 2:09 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Thomas Gleixner, Michael Kerrisk (man-pages), Darren Hart,
Torvald Riegel, Carlos O'Donell, Ingo Molnar, Jakub Jelinek,
linux-man, lkml, Arnd Bergmann, Steven Rostedt, Linux API,
Roland McGrath, Anton Blanchard, Eric Dumazet, bill o gallmeister,
Jan Kiszka, Daniel Wagner, Rich Felker, Andy Lutomirski,
bert hubert, Rusty Russell, Heinrich Schuchardt
In-Reply-To: <20150728204508.GG19282-ndre7Fmf5hadTX5a5knrm8zTDFooKrT+cvkQGrU6aU0@public.gmane.org>
On Tue, 2015-07-28 at 22:45 +0200, Peter Zijlstra wrote:
> Also, this code seems to use plist, which means it won't do the right
> thing for SCHED_DEADLINE either.
Ick, I don't look forward to seeing nice futex plists converted into
rbtrees. As opposed to, eg. rtmutexes, there are a few caveats:
- Dealing with the top_waiter in rtmutexes is always easy, but in
futexes we need to deal with keys, so caching the leftmost won't work as
nicely.
- This will bloat things like futex_wake, where O(logN) is not suited
for FIFO iteration. And iterating linked lists is, in essence, all that
we really do when calling futex(2).
I have to wonder about the extra overhead added by these points. I do
understand the dl concern, nonetheless.
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC 1/1] Documentation: describe how to add a system call
From: Randy Dunlap @ 2015-07-28 21:22 UTC (permalink / raw)
To: David Drysdale, linux-api-u79uwXL29TY76Z2rM5mHXA, Michael Kerrisk,
Andrew Morton, Arnd Bergmann, Shuah Khan, Jonathan Corbet
Cc: Andrea Arcangeli, Thomas Gleixner, Eric B Munson, Ingo Molnar,
H. Peter Anvin, Oleg Nesterov, Linus Torvalds, Greg Kroah-Hartman,
Andy Lutomirski, Al Viro, Rusty Russell, Peter Zijlstra,
Vivek Goyal, Alexei Starovoitov, David Herrmann,
Theodore Ts'o, Kees Cook, Miklos Szeredi, Milosz Tanski,
Fam Zheng, Josh Triplett, Mathieu Desnoyers,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-fy+rA21nqHI
In-Reply-To: <1438083663-24814-2-git-send-email-drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On 07/28/15 04:41, David Drysdale wrote:
> Add a document describing the process of adding a new system call,
> including the need for a flags argument for future compatibility, and
> covering 32-bit/64-bit concerns (albeit in an x86-centric way).
>
> Signed-off-by: David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Michael Kerrisk <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Nice job. and long overdue. Thanks.
Reviewed-by: Randy Dunlap <rdunlap-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
>
> ---
> Documentation/adding-syscalls.txt | 454 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 454 insertions(+)
> create mode 100644 Documentation/adding-syscalls.txt
>
> diff --git a/Documentation/adding-syscalls.txt b/Documentation/adding-syscalls.txt
> new file mode 100644
> index 000000000000..5f52edda8951
> --- /dev/null
> +++ b/Documentation/adding-syscalls.txt
> @@ -0,0 +1,454 @@
--
~Randy
^ permalink raw reply
* Re: [Xen-devel] [PATCH 0/8] Use correctly the Xen memory terminologies in Linux
From: Andrew Cooper @ 2015-07-28 21:12 UTC (permalink / raw)
To: H. Peter Anvin, Julien Grall, xen-devel
Cc: linux-fbdev, x86, netdev, Jiri Slaby, Thomas Gleixner,
Russell King, linux-scsi, Tomi Valkeinen, stefano.stabellini,
Ingo Molnar, linux-input, Jean-Christophe Plagniol-Villard,
ian.campbell, James E.J. Bottomley, Boris Ostrovsky,
linux-arm-kernel, Juergen Gross, Wei Liu, Greg Kroah-Hartman,
Dmitry Torokhov, linux-kernel, David Vrabel, linux-api,
linuxppc-dev
In-Reply-To: <55B7EECD.8050605@zytor.com>
On 28/07/15 22:06, H. Peter Anvin wrote:
> On 07/28/2015 08:02 AM, Julien Grall wrote:
>> Hi all,
>>
>> This patch series aims to use the memory terminologies described in
>> include/linux/mm.h [1] for Linux xen code.
>>
>> Linux is using mistakenly MFN when GFN is meant, I suspect this is because the
>> first support of Xen was for PV. This has brought some misimplementation
>> of memory helpers on ARM and make the developper confused about the expected
>> behavior.
>>
>> For instance, with pfn_to_mfn, we expect to get a MFN based on the name.
>> Although, if we look at the implementation on x86, it's returning a GFN.
>> Most of the callers are also using it this way.
>>
>> The first 2 patches of this series is ARM related in order to remove
>> PV specific helpers which should not be used and fixing the implementation of
>> pfn_to_mfn.
>>
>> The rest of the series is here rename most of the usage in the common code
>> of MFN to GFN. I also took the opportunity to replace most of the call to
>> pfn_to_gfn in the common code by page_to_gfn avoid construction such
>> as pfn_to_gfn(page_to_pfn(...).
>>
>> Note the one xen-blkfront will be dropped by 64K series [2], I can include it
>> if necessary.
>>
> Can we actually get some documentation for Xen before starting to change
> names around?
http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e758ed14f390342513405dd766e874934573e6cb
~Andrew
^ permalink raw reply
* Re: [PATCH 0/8] Use correctly the Xen memory terminologies in Linux
From: H. Peter Anvin @ 2015-07-28 21:06 UTC (permalink / raw)
To: Julien Grall, xen-devel
Cc: linux-fbdev, x86, netdev, Jiri Slaby, Thomas Gleixner,
Russell King, linux-scsi, Tomi Valkeinen, stefano.stabellini,
Ingo Molnar, linux-input, Jean-Christophe Plagniol-Villard,
ian.campbell, Konrad Rzeszutek Wilk, James E.J. Bottomley,
Boris Ostrovsky, linux-arm-kernel, Juergen Gross, Wei Liu,
Greg Kroah-Hartman, Dmitry Torokhov, linux-kernel, David Vrabel,
linux-api, linuxppc-dev
In-Reply-To: <1438095769-2560-1-git-send-email-julien.grall@citrix.com>
On 07/28/2015 08:02 AM, Julien Grall wrote:
> Hi all,
>
> This patch series aims to use the memory terminologies described in
> include/linux/mm.h [1] for Linux xen code.
>
> Linux is using mistakenly MFN when GFN is meant, I suspect this is because the
> first support of Xen was for PV. This has brought some misimplementation
> of memory helpers on ARM and make the developper confused about the expected
> behavior.
>
> For instance, with pfn_to_mfn, we expect to get a MFN based on the name.
> Although, if we look at the implementation on x86, it's returning a GFN.
> Most of the callers are also using it this way.
>
> The first 2 patches of this series is ARM related in order to remove
> PV specific helpers which should not be used and fixing the implementation of
> pfn_to_mfn.
>
> The rest of the series is here rename most of the usage in the common code
> of MFN to GFN. I also took the opportunity to replace most of the call to
> pfn_to_gfn in the common code by page_to_gfn avoid construction such
> as pfn_to_gfn(page_to_pfn(...).
>
> Note the one xen-blkfront will be dropped by 64K series [2], I can include it
> if necessary.
>
Can we actually get some documentation for Xen before starting to change
names around?
-hpa
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Thomas Gleixner @ 2015-07-28 21:03 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Michael Kerrisk (man-pages), Darren Hart, Torvald Riegel,
Carlos O'Donell, Ingo Molnar, Jakub Jelinek, linux-man, lkml,
Davidlohr Bueso, Arnd Bergmann, Steven Rostedt, Linux API,
Roland McGrath, Anton Blanchard, Eric Dumazet, bill o gallmeister,
Jan Kiszka, Daniel Wagner, Rich Felker, Andy Lutomirski,
bert hubert, Rusty Russell, Heinrich Schuchardt
In-Reply-To: <20150728204508.GG19282-ndre7Fmf5hadTX5a5knrm8zTDFooKrT+cvkQGrU6aU0@public.gmane.org>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1257 bytes --]
On Tue, 28 Jul 2015, Peter Zijlstra wrote:
> On Tue, Jul 28, 2015 at 10:23:51PM +0200, Thomas Gleixner wrote:
>
> > > FUTEX_WAKE (since Linux 2.6.0)
> > > This operation wakes at most val of the waiters that are
> > > waiting (e.g., inside FUTEX_WAIT) on the futex word at the
> > > address uaddr. Most commonly, val is specified as either
> > > 1 (wake up a single waiter) or INT_MAX (wake up all wait‐
> > > ers). No guarantee is provided about which waiters are
> > > awoken (e.g., a waiter with a higher scheduling priority
> > > is not guaranteed to be awoken in preference to a waiter
> > > with a lower priority).
> >
> > That's only correct up to Linux 2.6.21.
> >
> > Since 2.6.22 we have a priority ordered wakeup. For SCHED_OTHER
> > threads this takes the nice level into account. Threads with the same
> > priority are woken in FIFO order.
>
> Maybe don't mention the effects of SCHED_OTHER, order by nice value is
> 'wrong'.
Indeed.
> Also, this code seems to use plist, which means it won't do the right
> thing for SCHED_DEADLINE either.
>
> Do we want to go fix that?
I think so.
Thanks,
tglx
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Peter Zijlstra @ 2015-07-28 20:45 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Michael Kerrisk (man-pages), Darren Hart, Torvald Riegel,
Carlos O'Donell, Ingo Molnar, Jakub Jelinek, linux-man, lkml,
Davidlohr Bueso, Arnd Bergmann, Steven Rostedt, Linux API,
Roland McGrath, Anton Blanchard, Eric Dumazet, bill o gallmeister,
Jan Kiszka, Daniel Wagner, Rich Felker, Andy Lutomirski,
bert hubert, Rusty Russell, Heinrich Schuchardt
In-Reply-To: <alpine.DEB.2.11.1507282054370.3825@nanos>
On Tue, Jul 28, 2015 at 10:23:51PM +0200, Thomas Gleixner wrote:
> > FUTEX_WAKE (since Linux 2.6.0)
> > This operation wakes at most val of the waiters that are
> > waiting (e.g., inside FUTEX_WAIT) on the futex word at the
> > address uaddr. Most commonly, val is specified as either
> > 1 (wake up a single waiter) or INT_MAX (wake up all wait‐
> > ers). No guarantee is provided about which waiters are
> > awoken (e.g., a waiter with a higher scheduling priority
> > is not guaranteed to be awoken in preference to a waiter
> > with a lower priority).
>
> That's only correct up to Linux 2.6.21.
>
> Since 2.6.22 we have a priority ordered wakeup. For SCHED_OTHER
> threads this takes the nice level into account. Threads with the same
> priority are woken in FIFO order.
Maybe don't mention the effects of SCHED_OTHER, order by nice value is
'wrong'.
Also, this code seems to use plist, which means it won't do the right
thing for SCHED_DEADLINE either.
Do we want to go fix that?
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Thomas Gleixner @ 2015-07-28 20:23 UTC (permalink / raw)
To: Michael Kerrisk (man-pages)
Cc: Darren Hart, Torvald Riegel, Carlos O'Donell, Ingo Molnar,
Jakub Jelinek, linux-man, lkml, Davidlohr Bueso, Arnd Bergmann,
Steven Rostedt, Peter Zijlstra, Linux API, Roland McGrath,
Anton Blanchard, Eric Dumazet, bill o gallmeister, Jan Kiszka,
Daniel Wagner, Rich Felker, Andy Lutomirski, bert hubert,
Rusty Russell, Heinrich Schuchardt
In-Reply-To: <55B61EF3.7080302-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 46661 bytes --]
On Mon, 27 Jul 2015, Michael Kerrisk (man-pages) wrote:
> FUTEX_CLOCK_REALTIME (since Linux 2.6.28)
> This option bit can be employed only with the
> FUTEX_WAIT_BITSET and FUTEX_WAIT_REQUEUE_PI operations.
>
> If this option is set, the kernel treats timeout as an
> absolute time based on CLOCK_REALTIME.
>
> .\" FIXME XXX I added CLOCK_MONOTONIC below. Okay?
> If this option is not set, the kernel treats timeout as
> relative time, measured against the CLOCK_MONOTONIC clock.
That's correct.
> The operation specified in futex_op is one of the following:
>
> FUTEX_WAIT (since Linux 2.6.0)
> This operation tests that the value at the futex word
> pointed to by the address uaddr still contains the
> expected value val, and if so, then sleeps awaiting
> FUTEX_WAKE on the futex word. The load of the value of
> the futex word is an atomic memory access (i.e., using
> atomic machine instructions of the respective architec‐
> ture). This load, the comparison with the expected value,
> and starting to sleep are performed atomically and totally
> ordered with respect to other futex operations on the same
> futex word. If the thread starts to sleep, it is consid‐
> ered a waiter on this futex word. If the futex value does
> not match val, then the call fails immediately with the
> error EAGAIN.
>
> The purpose of the comparison with the expected value is
> to prevent lost wake-ups: If another thread changed the
> value of the futex word after the calling thread decided
> to block based on the prior value, and if the other thread
> executed a FUTEX_WAKE operation (or similar wake-up) after
> the value change and before this FUTEX_WAIT operation,
> then the latter will observe the value change and will not
> start to sleep.
>
> If the timeout argument is non-NULL, its contents specify
> a relative timeout for the wait, measured according to the
> .\" FIXME XXX I added CLOCK_MONOTONIC below. Okay?
Yes.
> CLOCK_MONOTONIC clock. (This interval will be rounded up
> to the system clock granularity, and kernel scheduling
> delays mean that the blocking interval may overrun by a
> small amount.)
The given wait time will be rounded up to the system
clock granularity and is guaranteed not to expire
early.
There are a gazillion reasons why it can expire late, but the
guarantee is that it never expires prematurely.
> If timeout is NULL, the call blocks indef‐
> initely.
Right.
> The arguments uaddr2 and val3 are ignored.
>
>
> FUTEX_WAKE (since Linux 2.6.0)
> This operation wakes at most val of the waiters that are
> waiting (e.g., inside FUTEX_WAIT) on the futex word at the
> address uaddr. Most commonly, val is specified as either
> 1 (wake up a single waiter) or INT_MAX (wake up all wait‐
> ers). No guarantee is provided about which waiters are
> awoken (e.g., a waiter with a higher scheduling priority
> is not guaranteed to be awoken in preference to a waiter
> with a lower priority).
That's only correct up to Linux 2.6.21.
Since 2.6.22 we have a priority ordered wakeup. For SCHED_OTHER
threads this takes the nice level into account. Threads with the same
priority are woken in FIFO order.
> The arguments timeout, uaddr2, and val3 are ignored.
>
> FUTEX_FD (from Linux 2.6.0 up to and including Linux 2.6.25)
> This operation creates a file descriptor that is associ‐
> ated with the futex at uaddr. The caller must close the
> returned file descriptor after use. When another process
> or thread performs a FUTEX_WAKE on the futex word, the
> file descriptor indicates as being readable with
> select(2), poll(2), and epoll(7)
>
> The file descriptor can be used to obtain asynchronous
> notifications: if val is nonzero, then when another
> process or thread executes a FUTEX_WAKE, the caller will
> receive the signal number that was passed in val.
>
> The arguments timeout, uaddr2 and val3 are ignored.
>
> .\" FIXME(Torvald) We never define "upped". Maybe just remove the
> .\" following sentence?
> To prevent race conditions, the caller should test if the
> futex has been upped after FUTEX_FD returns.
Yes, just remove it.
> Because it was inherently racy, FUTEX_FD has been removed
> from Linux 2.6.26 onward.
>
> FUTEX_REQUEUE (since Linux 2.6.0)
> .\" FIXME(Torvald) Is there some indication that FUTEX_REQUEUE is broken
> .\" in general, or is this comment implicitly speaking about the
> .\" condvar (?) use case? If the latter we might want to weaken the
> .\" advice below a little.
> .\" [Anyone else have input on this?]
The condvar use case exposes the flaw nicely, but that's pretty much
true for everything which wants a sane requeue operation.
> Avoid using this operation. It is broken for its intended
> purpose. Use FUTEX_CMP_REQUEUE instead.
>
> This operation performs the same task as
> FUTEX_CMP_REQUEUE, except that no check is made using the
> value in val3. (The argument val3 is ignored.)
>
> FUTEX_CMP_REQUEUE (since Linux 2.6.7)
> This operation first checks whether the location uaddr
> still contains the value val3. If not, the operation
> fails with the error EAGAIN. Otherwise, the operation
> wakes up a maximum of val waiters that are waiting on the
> futex at uaddr. If there are more than val waiters, then
> the remaining waiters are removed from the wait queue of
> the source futex at uaddr and added to the wait queue of
> the target futex at uaddr2. The val2 argument specifies
> an upper limit on the number of waiters that are requeued
> to the futex at uaddr2.
>
> .\" FIXME(Torvald) Is the following correct? Or is just the decision
> .\" which threads to wake or requeue part of the atomic operation?
>
> The load from uaddr is an atomic memory access (i.e.,
> using atomic machine instructions of the respective archi‐
> tecture). This load, the comparison with val3, and the
> requeueing of any waiters are performed atomically and
> totally ordered with respect to other operations on the
> same futex word.
It's atomic as the other atomic operations on the futex word. It's
always performed with the proper lock(s) held in the kernel. That
means any concurrent operation will serialize on that lock(s). User
space has to make sure, that depending on the observed value no
concurrent operations happen, but that's something the kernel cannot
control.
> This operation was added as a replacement for the earlier
> FUTEX_REQUEUE. The difference is that the check of the
> value at uaddr can be used to ensure that requeueing hap‐
> pens only under certain conditions. Both operations can
> be used to avoid a "thundering herd" effect when
> FUTEX_WAKE is used and all of the waiters that are woken
> need to acquire another futex.
>
> .\" FIXME Please review the following new paragraph to see if it is
> .\" accurate.
> Typical values to specify for val are 0 or or 1. (Speci‐
> fying INT_MAX is not useful, because it would make the
> FUTEX_CMP_REQUEUE operation equivalent to FUTEX_WAKE.)
> The limit value specified via val2 is typically either 1
> or INT_MAX. (Specifying the argument as 0 is not useful,
> because it would make the FUTEX_CMP_REQUEUE operation
> equivalent to FUTEX_WAIT.)
It's correct.
> .\" FIXME Here, it would be helpful to have an example of how
> .\" FUTEX_CMP_REQUEUE might be used, at the same time illustrating
> .\" why FUTEX_WAKE is unsuitable for the same use case.
Waiters:
lock(A)
while (!check_value(V)) {
unlock(A);
block_on(B);
lock(A);
};
unlock(A);
Note: B is a wait queue implemented with futexes.
If the waker would use FUTEX_WAKE and wake all waiters waiting on B
then those would all try to acquire lock A. That's called thundering
herd and pointless because all except one would immediately block on
lock A again.
Requeueing prevents that because it only wakes one waiter and moves
the other waiters to lock A. When that waiter unlocks A then the next
waiter can proceed ...
> FUTEX_WAKE_OP (since Linux 2.6.14)
> .\" FIXME I added a lengthy piece of text on FUTEX_WAKE_OP text,
> .\" and I'd be happy if someone checked it.
> .\"
> .\" FIXME(Torvald) The glibc condvar implementation is currently being
> .\" revised (e.g., to not use an internal lock anymore).
> .\" It is probably more future-proof to remove this paragraph.
> .\" [Torvald, do you have an update here?]
> .\"
> This operation was added to support some user-space use
> cases where more than one futex must be handled at the
> same time. The most notable example is the implementation
> of pthread_cond_signal(3), which requires operations on
> two futexes, the one used to implement the mutex and the
> one used in the implementation of the wait queue associ‐
> ated with the condition variable. FUTEX_WAKE_OP allows
> such cases to be implemented without leading to high rates
> of contention and context switching.
>
> The FUTEX_WAIT_OP operation is equivalent to executing the
> following code atomically and totally ordered with respect
> to other futex operations on any of the two supplied futex
> words:
>
> int oldval = *(int *) uaddr2;
> *(int *) uaddr2 = oldval op oparg;
> futex(uaddr, FUTEX_WAKE, val, 0, 0, 0);
> if (oldval cmp cmparg)
> futex(uaddr2, FUTEX_WAKE, val2, 0, 0, 0);
>
> In other words, FUTEX_WAIT_OP does the following:
>
> * saves the original value of the futex word at uaddr2
> and performs an operation to modify the value of the
> futex at uaddr2; this is an atomic read-modify-write
> memory access (i.e., using atomic machine instructions
> of the respective architecture)
>
> * wakes up a maximum of val waiters on the futex for the
> futex word at uaddr; and
>
> * dependent on the results of a test of the original
> value of the futex word at uaddr2, wakes up a maximum
> of val2 waiters on the futex for the futex word at
> uaddr2.
>
> The operation and comparison that are to be performed are
> encoded in the bits of the argument val3. Pictorially,
> the encoding is:
>
> +---+---+-----------+-----------+
> |op |cmp| oparg | cmparg |
> +---+---+-----------+-----------+
> 4 4 12 12 <== # of bits
>
> Expressed in code, the encoding is:
>
> #define FUTEX_OP(op, oparg, cmp, cmparg) \
> (((op & 0xf) << 28) | \
> ((cmp & 0xf) << 24) | \
> ((oparg & 0xfff) << 12) | \
> (cmparg & 0xfff))
>
> In the above, op and cmp are each one of the codes listed
> below. The oparg and cmparg components are literal
> numeric values, except as noted below.
>
> The op component has one of the following values:
>
> FUTEX_OP_SET 0 /* uaddr2 = oparg; */
> FUTEX_OP_ADD 1 /* uaddr2 += oparg; */
> FUTEX_OP_OR 2 /* uaddr2 |= oparg; */
> FUTEX_OP_ANDN 3 /* uaddr2 &= ~oparg; */
> FUTEX_OP_XOR 4 /* uaddr2 ^= oparg; */
>
> In addition, bit-wise ORing the following value into op
> causes (1 << oparg) to be used as the operand:
>
> FUTEX_OP_ARG_SHIFT 8 /* Use (1 << oparg) as operand */
>
> The cmp field is one of the following:
>
> FUTEX_OP_CMP_EQ 0 /* if (oldval == cmparg) wake */
> FUTEX_OP_CMP_NE 1 /* if (oldval != cmparg) wake */
> FUTEX_OP_CMP_LT 2 /* if (oldval < cmparg) wake */
> FUTEX_OP_CMP_LE 3 /* if (oldval <= cmparg) wake */
> FUTEX_OP_CMP_GT 4 /* if (oldval > cmparg) wake */
> FUTEX_OP_CMP_GE 5 /* if (oldval >= cmparg) wake */
>
> The return value of FUTEX_WAKE_OP is the sum of the number
> of waiters woken on the futex uaddr plus the number of
> waiters woken on the futex uaddr2.
>
> FUTEX_WAIT_BITSET (since Linux 2.6.25)
> This operation is like FUTEX_WAIT except that val3 is used
> to provide a 32-bit bitset to the kernel. This bitset is
> stored in the kernel-internal state of the waiter. See
> the description of FUTEX_WAKE_BITSET for further details.
>
> The FUTEX_WAIT_BITSET operation also interprets the time‐
> out argument differently from FUTEX_WAIT. See the discus‐
> sion of FUTEX_CLOCK_REALTIME, above.
>
> The uaddr2 argument is ignored.
>
> FUTEX_WAKE_BITSET (since Linux 2.6.25)
> This operation is the same as FUTEX_WAKE except that the
> val3 argument is used to provide a 32-bit bitset to the
> kernel. This bitset is used to select which waiters
> should be woken up. The selection is done by a bit-wise
> AND of the "wake" bitset (i.e., the value in val3) and the
> bitset which is stored in the kernel-internal state of the
> waiter (the "wait" bitset that is set using
> FUTEX_WAIT_BITSET). All of the waiters for which the
> result of the AND is nonzero are woken up; the remaining
> waiters are left sleeping.
>
> .\" FIXME XXX Is this next paragraph that I added okay?
> The effect of FUTEX_WAIT_BITSET and FUTEX_WAKE_BITSET is
> to allow selective wake-ups among multiple waiters that
> are blocked on the same futex. Note, however, that using
> this bitset multiplexing feature on a futex is less effi‐
> cient than simply using multiple futexes, because employ‐
s/is less efficient/can be less efficient/
It really depends on the usecase.
> ing bitset multiplexing requires the kernel to check all
> waiters on a futex, including those that are not inter‐
> ested in being woken up (i.e., they do not have the rele‐
> vant bit set in their "wait" bitset).
>
> The uaddr2 and timeout arguments are ignored.
>
> The FUTEX_WAIT and FUTEX_WAKE operations correspond to
> FUTEX_WAIT_BITSET and FUTEX_WAKE_BITSET operations where
> the bitsets are all ones.
>
> Priority-inheritance futexes
> Linux supports priority-inheritance (PI) futexes in order to han‐
> dle priority-inversion problems that can be encountered with nor‐
> mal futex locks. Priority inversion is the problem that occurs
> when a high-priority task is blocked waiting to acquire a lock
> held by a low-priority task, while tasks at an intermediate pri‐
> ority continuously preempt the low-priority task from the CPU.
> Consequently, the low-priority task makes no progress toward
> releasing the lock, and the high-priority task remains blocked.
>
> Priority inheritance is a mechanism for dealing with the prior‐
> ity-inversion problem. With this mechanism, when a high-priority
> task becomes blocked by a lock held by a low-priority task, the
> latter's priority is temporarily raised to that of the former, so
> that it is not preempted by any intermediate level tasks, and can
> thus make progress toward releasing the lock. To be effective,
> priority inheritance must be transitive, meaning that if a high-
> priority task blocks on a lock held by a lower-priority task that
> is itself blocked by lock held by another intermediate-priority
> task (and so on, for chains of arbitrary length), then both of
> those task (or more generally, all of the tasks in a lock chain)
> have their priorities raised to be the same as the high-priority
> task.
>
> .\" FIXME XXX The following is my attempt at a definition of PI futexes,
> .\" based on mail discussions with Darren Hart. Does it seem okay?
>
> From a user-space perspective, what makes a futex PI-aware is a
> policy agreement between user space and the kernel about the
> value of the futex word (described in a moment), coupled with the
> use of the PI futex operations described below (in particular,
> FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, and FUTEX_CMP_REQUEUE_PI).
>
> .\" FIXME XXX ===== Start of adapted Hart/Guniguntala text =====
> .\" The following text is drawn from the Hart/Guniguntala paper
> .\" (listed in SEE ALSO), but I have reworded some pieces
> .\" significantly. Please check it.
>
> The PI futex operations described below differ from the other
> futex operations in that they impose policy on the use of the
> value of the futex word:
>
> * If the lock is not acquired, the futex word's value shall be
> 0.
>
> * If the lock is acquired, the futex word's value shall be the
> thread ID (TID; see gettid(2)) of the owning thread.
>
> * If the lock is owned and there are threads contending for the
> lock, then the FUTEX_WAITERS bit shall be set in the futex
> word's value; in other words, this value is:
>
> FUTEX_WAITERS | TID
>
>
> Note that a PI futex word never just has the value FUTEX_WAITERS,
> which is a permissible state for non-PI futexes.
>
> With this policy in place, a user-space application can acquire a
> not-acquired lock or release a lock that no other threads try to
> acquire using atomic instructions executed in user space (e.g., a
> compare-and-swap operation such as cmpxchg on the x86 architec‐
> ture). Acquiring a lock simply consists of using compare-and-
> swap to atomically set the futex word's value to the caller's TID
> if its previous value was 0. Releasing a lock requires using
> compare-and-swap to set the futex word's value to 0 if the previ‐
> ous value was the expected TID.
>
> If a futex is already acquired (i.e., has a nonzero value), wait‐
> ers must employ the FUTEX_LOCK_PI operation to acquire the lock.
> If other threads are waiting for the lock, then the FUTEX_WAITERS
> bit is set in the futex value; in this case, the lock owner must
> employ the FUTEX_UNLOCK_PI operation to release the lock.
>
> In the cases where callers are forced into the kernel (i.e.,
> required to perform a futex() call), they then deal directly with
> a so-called RT-mutex, a kernel locking mechanism which implements
> the required priority-inheritance semantics. After the RT-mutex
> is acquired, the futex value is updated accordingly, before the
> calling thread returns to user space.
> .\" FIXME ===== End of adapted Hart/Guniguntala text =====
That's correct.
> .\" FIXME We need some explanation in the following paragraph of *why*
> .\" it is important to note that "the kernel will update the
> .\" futex word's value prior
> It is important to note to returning to user space" . Can someone
> explain? that the kernel will update the futex word's value
> prior to returning to user space. Unlike the other futex opera‐
> tions described above, the PI futex operations are designed for
> the implementation of very specific IPC mechanisms.
If there are multiple waiters on a pi futex then a wake pi operation
will wake the first waiter and hand over the lock to this waiter. This
includes handing over the rtmutex which represents the futex in the
kernel. The strict requirement is that the futex owner and the rtmutex
owner must be the same, except for the update period which is
serialized by the futex internal locking. That means the kernel must
update the user space value prior to returning to user space.
> .\"
> .\" FIXME XXX In discussing errors for FUTEX_CMP_REQUEUE_PI, Darren Hart
> .\" made the observation that "EINVAL is returned if the non-pi
> .\" to pi or op pairing semantics are violated."
> .\" Probably there needs to be a general statement about this
> .\" requirement, probably located at about this point in the page.
> .\" Darren (or someone else), care to take a shot at this?
Well, that's hard to describe because the kernel only has a limited
way of detecting such mismatches. It only can detect it when there are
non PI waiters on a futex and a PI function is called or vice versa.
> .\" FIXME Somewhere on this page (I guess under the discussion of PI
> .\" futexes) we need a discussion of the FUTEX_OWNER_DIED bit.
> .\" Can someone propose a text?
If a futex has a rtmutex associated in the kernel, i.e. when there are
blocked waiters, and the owner of the futex/rtmutex dies unexpectedly,
then the kernel cleans up the rtmutex (as it holds a reference to the
dying task) and hands it over to the next waiter. That requires that
the user space value is updated accordingly. The kernel sets the
FUTEX_OWNER_DIED in the user space value along with the TID of the new
owner. User space is responsible for cleaning this up, though there
are cases where the kernel does the cleanup.
The FUTEX_OWNER_DIED bit can also be set on uncontended futexes, where
the kernel has no state associated. This happens via the robust futex
mechanism. In that case the futex value will be set to
FUTEX_OWNER_DIED. The robust futex mechanism is also available for non
PI futexes.
> PI futexes are operated on by specifying one of the following
> values in futex_op:
>
> FUTEX_LOCK_PI (since Linux 2.6.18)
> .\" FIXME I did some significant rewording of tglx's text to create
> .\" the text below.
> .\" Please check the following paragraph, in case I injected
> .\" errors.
> This operation is used after after an attempt to acquire
> the lock via an atomic user-space instruction failed
> because the futex word has a nonzero value—specifically,
> because it contained the namespace-specific TID of the
> lock owner.
> .\" FIXME In the preceding line, what does "namespace-specific" mean?
> .\" (I kept those words from tglx.)
> .\" That is, what kind of namespace are we talking about?
> .\" (I suppose we are talking PID namespaces here, but I want to
> .\" be sure.)
Yes.
> The operation checks the value of the futex word at the
> address uaddr. If the value is 0, then the kernel tries
> to atomically set the futex value to the caller's TID.
> .\" FIXME What would be the cause(s) of failure referred to
> .\" in the following sentence?
> If
> that fails, or the futex word's value is nonzero, the ker‐
'If that fails' does not make sense. If the user space access fails we
return -EFAULT and let user space deal with the mess.
The operation here is similar to the FUTEX_WAIT logic. When the user
space atomic acquire does not succeed because the futex value was non
zero, then the waiter goes into the kernel, takes the kernel internal
lock and retries the acquisition under the lock. If the acquisition
does not succeed either, then it sets the FUTEX_WAITERS bit, to signal
the lock owner that it needs to go into the kernel. Here is the pseudo
code:
lock(kernel_lock);
retry:
/*
* Owner might have unlocked in userspace before we
* were able to set the waiter bit.
*/
if (atomic_acquire(futex) == SUCCESS) {
unlock(kernel_lock());
return 0;
}
/*
* Owner might have unlocked after the above atomic_acquire()
* attempt.
*/
if (atomic_set_waiters_bit(futex) != SUCCESS)
goto retry;
queue_waiter();
unlock(kernel_lock);
block();
> nel atomically sets the FUTEX_WAITERS bit, which signals
> the futex owner that it cannot unlock the futex in user
> space atomically by setting the futex value to 0. After
> that, the kernel tries to find the thread which is associ‐
> ated with the owner TID, creates or reuses kernel state on
> behalf of the owner and attaches the waiter to it.
> .\" FIXME Could I get a bit more detail on the previous lines?
> .\" What is "creates or reuses kernel state" about?
> .\" (I think this needs to be clearer in the page)
If this is the first waiter then there is no kernel state for this
futex, so it is created. That means the rtmutex is locked and the
futex owner established as the owner of the rtmutex. If there is a
waiter, then the state is reused, i.e. the new waiter is enqueued into
the rtmutex waiter list.
> .\" FIXME In the next line, what type of "priority" are we talking about?
> .\" Realtime priorities for SCHED_FIFO and SCHED_RR?
> .\" Or something else?
>
> The
> enqueueing of the waiter is in descending priority order
> if more than one waiter exists.
That also covers sched deadline.
> .\" FIXME In the next sentence, what type of "priority" are we talking about?
> .\" Realtime priorities for SCHED_FIFO and SCHED_RR?
> .\" Or something else?
> .\" FIXME What does "bandwidth" refer to in the next sentence?
>
> The owner inherits either
> the priority or the bandwidth of the waiter.
If the highest priority waiter is SCHED_DEADLINE, then the owner
inherits cpu bandwidth from the waiter as there is no priority
associated to SCHED_DEADLINE tasks.
If the highest priority waiter is SCHED_FIFO/RR, then the owner
inherits the waiter priority.
> .\" FIXME In the preceding sentence, what determines whether the
> .\" owner inherits the priority versus the bandwidth?
>
> .\" FIXME Could I get some help translating the next sentence into
> .\" something that user-space developers (and I) can understand?
> .\" In particular, what are "nested locks" in this context?
>
> This inheri‐
> tance follows the lock chain in the case of nested locking
> and performs deadlock detection.
T1 blocks on lock A held by T2
T2 blocks on lock B held by T3
So we have a lock chain A, B. The inheritance mechanism follows the
lock chain and propagates the highest waiter priority up to the end of
the chain.
> .\" FIXME tglx said "The timeout argument is handled as described in
> .\" FUTEX_WAIT." However, it appears to me that this is not right.
> .\" Is the following formulation correct?
> The timeout argument provides a timeout for the lock
> attempt. It is interpreted as an absolute time, measured
> against the CLOCK_REALTIME clock. If timeout is NULL, the
> operation will block indefinitely.
Indeed.
> The uaddr2, val, and val3 arguments are ignored.
>
> FUTEX_TRYLOCK_PI (since Linux 2.6.18)
> .\" FIXME I think it would be helpful here to say a few more words about
> .\" the difference(s) between FUTEX_LOCK_PI and FUTEX_TRYLOCK_PI.
> .\" Can someone propose something?
> This operation tries to acquire the futex at uaddr. It
> deals with the situation where the TID value at uaddr is
> 0, but the FUTEX_WAITERS bit is set. User space cannot
> handle this condition in a race-free manner
> .\" FIXME How does the situation in the previous sentence come about?
> .\" Probably it would be helpful to say something about that in
> .\" the man page.
> .\" FIXME And *how* does FUTEX_TRYLOCK_PI deal with this situation?
That should be expressed differently:
This operation tries to acquire the futex at uaddr. It's
invoked when the user space atomic acquire did not
succeed because the user space value was not 0.
The trylock in kernel might succeed because the user space
value contains stale state (FUTEX_WAITERS and or
FUTEX_OWNER_DIED). This can happen when the owner of the
futex died.
> The uaddr2, val, timeout, and val3 arguments are ignored.
>
> FUTEX_UNLOCK_PI (since Linux 2.6.18)
> This operation wakes the top priority waiter that is wait‐
> ing in FUTEX_LOCK_PI on the futex address provided by the
> uaddr argument.
>
> This is called when the user space value at uaddr cannot
> be changed atomically from a TID (of the owner) to 0.
>
> The uaddr2, val, timeout, and val3 arguments are ignored.
>
> FUTEX_CMP_REQUEUE_PI (since Linux 2.6.31)
> This operation is a PI-aware variant of FUTEX_CMP_REQUEUE.
> It requeues waiters that are blocked via
> FUTEX_WAIT_REQUEUE_PI on uaddr from a non-PI source futex
> (uaddr) to a PI target futex (uaddr2).
>
> As with FUTEX_CMP_REQUEUE, this operation wakes up a maxi‐
> mum of val waiters that are waiting on the futex at uaddr.
> However, for FUTEX_CMP_REQUEUE_PI, val is required to be 1
> (since the main point is to avoid a thundering herd). The
> remaining waiters are removed from the wait queue of the
> source futex at uaddr and added to the wait queue of the
> target futex at uaddr2.
>
> The val2 and val3 arguments serve the same purposes as for
> FUTEX_CMP_REQUEUE.
> .\" FIXME The page at http://locklessinc.com/articles/futex_cheat_sheet/
> .\" notes that "priority-inheritance Futex to priority-inheritance
> .\" Futex requeues are currently unsupported". Do we need to say
> .\" something in the man page about that?
>
And they never will be supported because they make no sense at all.
>
> FUTEX_WAIT_REQUEUE_PI (since Linux 2.6.31)
>
> .\" FIXME I find the next sentence (from tglx) pretty hard to grok.
> .\" Could someone explain it a bit more?
>
> Wait operation to wait on a non-PI futex at uaddr and
> potentially be requeued onto a PI futex at uaddr2. The
> wait operation on uaddr is the same as FUTEX_WAIT.
let me copy the pseudo code from cmp_requeue
lock(A)
while (!check_value(V)) {
unlock(A);
block_on(B);
lock(A);
};
unlock(A);
So in this case B is the non-PI futex (the wait queue) and A is a PI
futex. So wait operation on B is the same as in FUTEX_WAIT.
>
> .\" FIXME I'm not quite clear on the meaning of the following sentence.
> .\" Is this trying to say that while blocked in a
> .\" FUTEX_WAIT_REQUEUE_PI, it could happen that another
> .\" task does a FUTEX_WAKE on uaddr that simply causes
> .\" a normal wake, with the result that the FUTEX_WAIT_REQUEUE_PI
> .\" does not complete? What happens then to the FUTEX_WAIT_REQUEUE_PI
> .\" opertion? Does it remain blocked, or does it unblock
> .\" In which case, what does user space see?
It unblocks and returns -EWOULDBLOCK.
> The
> waiter can be removed from the wait on uaddr via
> FUTEX_WAKE without requeueing on uaddr2.
> .\" FIXME Please check the following. tglx said "The timeout argument
> .\" is handled as described in FUTEX_WAIT.", but the truth is
> .\" as below, AFAICS
>
> If timeout is not NULL, it specifies a timeout for the
> wait operation; this timeout is interpreted as outlined
> above in the description of the FUTEX_CLOCK_REALTIME
> option. If timeout is NULL, the operation can block
> indefinitely.
>
> The val3 argument is ignored.
Correct
> .\" FIXME Re the preceding sentence... Actually 'val3' is internally set to
> .\" FUTEX_BITSET_MATCH_ANY before calling futex_wait_requeue_pi().
> .\" I'm not sure we need to say anything about this though.
> .\" Comments?
That's a kernel internal and can be removed
>
> The FUTEX_WAIT_REQUEUE_PI and FUTEX_CMP_REQUEUE_PI were
> added to support a fairly specific use case: support for
> priority-inheritance-aware POSIX threads condition vari‐
> ables. The idea is that these operations should always be
> paired, in order to ensure that user space and the kernel
> remain in sync. Thus, in the FUTEX_WAIT_REQUEUE_PI opera‐
> tion, the user-space application pre-specifies the target
> of the requeue that takes place in the
> FUTEX_CMP_REQUEUE_PI operation.
>
> RETURN VALUE
> In the event of an error (and assuming that futex() was invoked
> via syscall(2)), all operations return -1 and set errno to indi‐
> cate the cause of the error. The return value on success depends
> on the operation, as described in the following list:
>
> FUTEX_WAIT
> Returns 0 if the caller was woken up. Note that a wake-up
> can also be caused by common futex usage patterns in unre‐
> lated code that happened to have previously used the futex
> word's memory location (e.g., typical futex-based imple‐
> mentations of Pthreads mutexes can cause this under some
> conditions). Therefore, callers should always conserva‐
> tively assume that a return value of 0 can mean a spurious
> wake-up, and use the futex word's value (i.e., the user
> space synchronization scheme)
> to decide whether to continue to block or not.
>
> FUTEX_WAKE
> Returns the number of waiters that were woken up.
>
> FUTEX_FD
> Returns the new file descriptor associated with the futex.
>
> FUTEX_REQUEUE
> Returns the number of waiters that were woken up.
>
> FUTEX_CMP_REQUEUE
> Returns the total number of waiters that were woken up or
> requeued to the futex for the futex word at uaddr2. If
> this value is greater than val, then difference is the
> number of waiters requeued to the futex for the futex word
> at uaddr2.
>
> FUTEX_WAKE_OP
> Returns the total number of waiters that were woken up.
> This is the sum of the woken waiters on the two futexes
> for the futex words at uaddr and uaddr2.
>
> FUTEX_WAIT_BITSET
> Returns 0 if the caller was woken up. See FUTEX_WAIT for
> how to interpret this correctly in practice.
>
> FUTEX_WAKE_BITSET
> Returns the number of waiters that were woken up.
>
> FUTEX_LOCK_PI
> Returns 0 if the futex was successfully locked.
>
> FUTEX_TRYLOCK_PI
> Returns 0 if the futex was successfully locked.
>
> FUTEX_UNLOCK_PI
> Returns 0 if the futex was successfully unlocked.
>
> FUTEX_CMP_REQUEUE_PI
> Returns the total number of waiters that were woken up or
> requeued to the futex for the futex word at uaddr2. If
> this value is greater than val, then difference is the
> number of waiters requeued to the futex for the futex word
> at uaddr2.
>
> FUTEX_WAIT_REQUEUE_PI
> Returns 0 if the caller was successfully requeued to the
> futex for the futex word at uaddr2.
>
> ERRORS
> EACCES No read access to the memory of a futex word.
>
> EAGAIN (FUTEX_WAIT, FUTEX_WAIT_BITSET, FUTEX_WAIT_REQUEUE_PI) The
> value pointed to by uaddr was not equal to the expected
> value val at the time of the call.
>
> Note: on Linux, the symbolic names EAGAIN and EWOULDBLOCK
> (both of which appear in different parts of the kernel
> futex code) have the same value.
>
> EAGAIN (FUTEX_CMP_REQUEUE, FUTEX_CMP_REQUEUE_PI) The value
> pointed to by uaddr is not equal to the expected value
> val3. (This probably indicates a race; use the safe
> FUTEX_WAKE now.)
> .\" FIXME: Is the preceding sentence "(This probably...") correct?
> .\" [I would prefer to remove this sentence. --triegel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
This part should be removed:
"(This probably indicates a race; use the safe FUTEX_WAKE now.)
>
> EAGAIN (FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, FUTEX_CMP_REQUEUE_PI)
> The futex owner thread ID of uaddr (for
> FUTEX_CMP_REQUEUE_PI: uaddr2) is about to exit, but has
> not yet handled the internal state cleanup. Try again.
>
> .\" FIXME XXX Should there be an EAGAIN case for FUTEX_TRYLOCK_PI?
> .\" It seems so, looking at the handling of the rt_mutex_trylock()
> .\" call in futex_lock_pi()
> .\" (Davidlohr also thinks so.)
Yes. It's the same internal logic so it can return EAGAIN
>
> EDEADLK
> (FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, FUTEX_CMP_REQUEUE_PI)
> The futex word at uaddr is already locked by the caller.
>
> EDEADLK
>
> .\" FIXME I reworded tglx's text somewhat; is the following okay?
>
> (FUTEX_CMP_REQUEUE_PI) While requeueing a waiter to the PI
> futex for the futex word at uaddr2, the kernel detected a
> deadlock.
Yes
>
> .\" FIXME XXX I see that kernel/locking/rtmutex.c uses EDEADLK in some
> .\" places, and EDEADLOCK in others. On almost all architectures
> .\" these constants are synonymous. Is there a reason that both
> .\" names are used?
No. We should probably fix that.
> EFAULT A required pointer argument (i.e., uaddr, uaddr2, or time‐
> out) did not point to a valid user-space address.
>
> EINTR A FUTEX_WAIT or FUTEX_WAIT_BITSET operation was inter‐
> rupted by a signal (see signal(7)). In kernels before
> Linux 2.6.22, this error could also be returned for on a
> spurious wakeup; since Linux 2.6.22, this no longer hap‐
> pens.
>
> EINVAL The operation in futex_op is one of those that employs a
> timeout, but the supplied timeout argument was invalid
> (tv_sec was less than zero, or tv_nsec was not less than
> 1,000,000,000).
>
> EINVAL The operation specified in futex_op employs one or both of
> the pointers uaddr and uaddr2, but one of these does not
> point to a valid object—that is, the address is not four-
> byte-aligned.
>
> EINVAL (FUTEX_WAIT_BITSET, FUTEX_WAKE_BITSET) The bitset supplied
> in val3 is zero.
>
> EINVAL (FUTEX_CMP_REQUEUE_PI) uaddr equals uaddr2 (i.e., an
> attempt was made to requeue to the same futex).
>
> EINVAL (FUTEX_FD) The signal number supplied in val is invalid.
>
> EINVAL (FUTEX_WAKE, FUTEX_WAKE_OP, FUTEX_WAKE_BITSET,
> FUTEX_REQUEUE, FUTEX_CMP_REQUEUE) The kernel detected an
> inconsistency between the user-space state at uaddr and
> the kernel state—that is, it detected a waiter which waits
> in FUTEX_LOCK_PI on uaddr.
>
> EINVAL (FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, FUTEX_UNLOCK_PI) The
> kernel detected an inconsistency between the user-space
> state at uaddr and the kernel state. This indicates
> either state corruption or that the kernel found a waiter
> on uaddr which is waiting via FUTEX_WAIT or
> FUTEX_WAIT_BITSET.
> .\" FIXME Above, tglx did not mention the "state corruption" case for
> .\" FUTEX_UNLOCK_PI, but I have added it, since I'm estimating
> .\" that it also applied for FUTEX_UNLOCK_PI.
> .\" So, does that case also apply for FUTEX_UNLOCK_PI?
Yes
>
> EINVAL (FUTEX_CMP_REQUEUE_PI) The kernel detected an inconsis‐
> tency between the user-space state at uaddr2 and the ker‐
> nel state; that is, the kernel detected a waiter which
> waits via FUTEX_WAIT on uaddr2.
> .\" FIXME In the preceding sentence, tglx did not mention FUTEX_WAIT_BITSET,
> .\" but should that not also be included here?
Yes
>
> EINVAL (FUTEX_CMP_REQUEUE_PI) The kernel detected an inconsis‐
> tency between the user-space state at uaddr and the kernel
> state; that is, the kernel detected a waiter which waits
> via FUTEX_WAIT or FUTEX_WAIT_BITESET on uaddr.
>
> EINVAL (FUTEX_CMP_REQUEUE_PI) The kernel detected an inconsis‐
> tency between the user-space state at uaddr and the kernel
> state; that is, the kernel detected a waiter which waits
> on uaddr via FUTEX_LOCK_PI (instead of
> FUTEX_WAIT_REQUEUE_PI).
>
> .\" FIXME XXX The following is a reworded version of Darren Hart's text.
> .\" Please check that I did not introduce any errors.
> EINVAL (FUTEX_CMP_REQUEUE_PI) An attempt was made to requeue a
> waiter to a futex other than that specified by the match‐
> ing FUTEX_WAIT_REQUEUE_PI call for that waiter.
Correct. That handles the case:
wait_requeue_pi(A, B);
requeue_pi(A, C);
> EINVAL (FUTEX_CMP_REQUEUE_PI) The val argument is not 1.
>
> EINVAL Invalid argument.
>
> ENOMEM (FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, FUTEX_CMP_REQUEUE_PI)
> The kernel could not allocate memory to hold state infor‐
> mation.
>
> ENFILE (FUTEX_FD) The system limit on the total number of open
> files has been reached.
>
> ENOSYS Invalid operation specified in futex_op.
>
> ENOSYS The FUTEX_CLOCK_REALTIME option was specified in futex_op,
> but the accompanying operation was neither FUTEX_WAIT_BIT‐
> SET nor FUTEX_WAIT_REQUEUE_PI.
>
> ENOSYS (FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, FUTEX_UNLOCK_PI,
> FUTEX_CMP_REQUEUE_PI, FUTEX_WAIT_REQUEUE_PI) A run-time
> check determined that the operation is not available. The
> PI futex operations are not implemented on all architec‐
> tures and are not supported on some CPU variants.
>
> EPERM (FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, FUTEX_CMP_REQUEUE_PI)
> The caller is not allowed to attach itself to the futex at
> uaddr (for FUTEX_CMP_REQUEUE_PI: the futex at uaddr2).
> (This may be caused by a state corruption in user space.)
>
> EPERM (FUTEX_UNLOCK_PI) The caller does not own the lock repre‐
> sented by the futex word.
>
> ESRCH (FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, FUTEX_CMP_REQUEUE_PI)
>
> .\" FIXME I reworded the following sentence a bit differently from
> .\" tglx's formulation. Is it okay?
>
> The thread ID in the futex word at uaddr does not exist.
Right.
> ESRCH (FUTEX_CMP_REQUEUE_PI)
>
> .\" FIXME I reworded the following sentence a bit differently from
> .\" tglx's formulation. Is it okay?
>
> The thread ID in the futex word at
> uaddr2 does not exist.
Right
Thanks,
tglx
^ permalink raw reply
* [PATCH v5 4/6] cpu_isolated: provide strict mode configurable signal
From: Chris Metcalf @ 2015-07-28 19:49 UTC (permalink / raw)
To: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
Viresh Kumar, Catalin Marinas, Will Deacon, linux-doc, linux-api,
linux-kernel
Cc: Chris Metcalf
In-Reply-To: <1438112980-9981-1-git-send-email-cmetcalf@ezchip.com>
Allow userspace to override the default SIGKILL delivered
when a cpu_isolated process in STRICT mode does a syscall
or otherwise synchronously enters the kernel.
In addition to being able to set the signal, we now also
pass whether or not the interruption was from a syscall in
the si_code field of the siginfo.
Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
---
include/uapi/linux/prctl.h | 2 ++
kernel/time/cpu_isolated.c | 17 ++++++++++++-----
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 0c11238a84fb..ab45bd3d5799 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -195,5 +195,7 @@ struct prctl_mm_map {
#define PR_GET_CPU_ISOLATED 48
# define PR_CPU_ISOLATED_ENABLE (1 << 0)
# define PR_CPU_ISOLATED_STRICT (1 << 1)
+# define PR_CPU_ISOLATED_SET_SIG(sig) (((sig) & 0x7f) << 8)
+# define PR_CPU_ISOLATED_GET_SIG(bits) (((bits) >> 8) & 0x7f)
#endif /* _LINUX_PRCTL_H */
diff --git a/kernel/time/cpu_isolated.c b/kernel/time/cpu_isolated.c
index d30bf3852897..9f8fcbd97770 100644
--- a/kernel/time/cpu_isolated.c
+++ b/kernel/time/cpu_isolated.c
@@ -71,11 +71,18 @@ void cpu_isolated_enter(void)
}
}
-static void kill_cpu_isolated_strict_task(void)
-{
+static void kill_cpu_isolated_strict_task(int is_syscall)
+ {
+ siginfo_t info = {};
+ int sig;
+
dump_stack();
current->cpu_isolated_flags &= ~PR_CPU_ISOLATED_ENABLE;
- send_sig(SIGKILL, current, 1);
+
+ sig = PR_CPU_ISOLATED_GET_SIG(current->cpu_isolated_flags) ?: SIGKILL;
+ info.si_signo = sig;
+ info.si_code = is_syscall;
+ send_sig_info(sig, &info, current);
}
/*
@@ -94,7 +101,7 @@ void cpu_isolated_syscall(int syscall)
pr_warn("%s/%d: cpu_isolated strict mode violated by syscall %d\n",
current->comm, current->pid, syscall);
- kill_cpu_isolated_strict_task();
+ kill_cpu_isolated_strict_task(1);
}
/*
@@ -105,5 +112,5 @@ void cpu_isolated_exception(void)
{
pr_warn("%s/%d: cpu_isolated strict mode violated by exception\n",
current->comm, current->pid);
- kill_cpu_isolated_strict_task();
+ kill_cpu_isolated_strict_task(0);
}
--
2.1.2
^ permalink raw reply related
* [PATCH v5 3/6] cpu_isolated: support PR_CPU_ISOLATED_STRICT mode
From: Chris Metcalf @ 2015-07-28 19:49 UTC (permalink / raw)
To: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
Viresh Kumar, Catalin Marinas, Will Deacon, linux-doc, linux-api,
linux-kernel
Cc: Chris Metcalf
In-Reply-To: <1438112980-9981-1-git-send-email-cmetcalf@ezchip.com>
With cpu_isolated mode, the task is in principle guaranteed not to be
interrupted by the kernel, but only if it behaves. In particular,
if it enters the kernel via system call, page fault, or any of a
number of other synchronous traps, it may be unexpectedly exposed
to long latencies. Add a simple flag that puts the process into
a state where any such kernel entry is fatal.
To allow the state to be entered and exited, we ignore the prctl()
syscall so that we can clear the bit again later, and we ignore
exit/exit_group to allow exiting the task without a pointless signal
killing you as you try to do so.
This change adds the syscall-detection hooks only for x86, arm64,
and tile.
The signature of context_tracking_exit() changes to report whether
we, in fact, are exiting back to user space, so that we can track
user exceptions properly separately from other kernel entries.
Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
---
Note: Andy Lutomirski points out that improvements are coming
to the context_tracking code to make it more robust, which may
mean that some of the code suggested here for context_tracking
may not be necessary. I am keeping it in the series for now since
it is required for it to work based on 4.2-rc3.
arch/arm64/kernel/ptrace.c | 5 +++++
arch/tile/kernel/ptrace.c | 5 ++++-
arch/x86/kernel/ptrace.c | 2 ++
include/linux/context_tracking.h | 11 ++++++++---
include/linux/cpu_isolated.h | 16 ++++++++++++++++
include/uapi/linux/prctl.h | 1 +
kernel/context_tracking.c | 9 ++++++---
kernel/time/cpu_isolated.c | 38 ++++++++++++++++++++++++++++++++++++++
8 files changed, 80 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index d882b833dbdb..ff83968ab4d4 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -37,6 +37,7 @@
#include <linux/regset.h>
#include <linux/tracehook.h>
#include <linux/elf.h>
+#include <linux/cpu_isolated.h>
#include <asm/compat.h>
#include <asm/debug-monitors.h>
@@ -1150,6 +1151,10 @@ static void tracehook_report_syscall(struct pt_regs *regs,
asmlinkage int syscall_trace_enter(struct pt_regs *regs)
{
+ /* Ensure we report cpu_isolated violations in all circumstances. */
+ if (test_thread_flag(TIF_NOHZ) && cpu_isolated_strict())
+ cpu_isolated_syscall(regs->syscallno);
+
/* Do the secure computing check first; failures should be fast. */
if (secure_computing() == -1)
return -1;
diff --git a/arch/tile/kernel/ptrace.c b/arch/tile/kernel/ptrace.c
index f84eed8243da..e54256c54311 100644
--- a/arch/tile/kernel/ptrace.c
+++ b/arch/tile/kernel/ptrace.c
@@ -259,8 +259,11 @@ int do_syscall_trace_enter(struct pt_regs *regs)
* If TIF_NOHZ is set, we are required to call user_exit() before
* doing anything that could touch RCU.
*/
- if (work & _TIF_NOHZ)
+ if (work & _TIF_NOHZ) {
user_exit();
+ if (cpu_isolated_strict())
+ cpu_isolated_syscall(regs->regs[TREG_SYSCALL_NR]);
+ }
if (work & _TIF_SYSCALL_TRACE) {
if (tracehook_report_syscall_entry(regs))
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 9be72bc3613f..e5aec57e8e25 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1479,6 +1479,8 @@ unsigned long syscall_trace_enter_phase1(struct pt_regs *regs, u32 arch)
if (work & _TIF_NOHZ) {
user_exit();
work &= ~_TIF_NOHZ;
+ if (cpu_isolated_strict())
+ cpu_isolated_syscall(regs->orig_ax);
}
#ifdef CONFIG_SECCOMP
diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
index b96bd299966f..590414ef2bf1 100644
--- a/include/linux/context_tracking.h
+++ b/include/linux/context_tracking.h
@@ -3,6 +3,7 @@
#include <linux/sched.h>
#include <linux/vtime.h>
+#include <linux/cpu_isolated.h>
#include <linux/context_tracking_state.h>
#include <asm/ptrace.h>
@@ -11,7 +12,7 @@
extern void context_tracking_cpu_set(int cpu);
extern void context_tracking_enter(enum ctx_state state);
-extern void context_tracking_exit(enum ctx_state state);
+extern bool context_tracking_exit(enum ctx_state state);
extern void context_tracking_user_enter(void);
extern void context_tracking_user_exit(void);
@@ -35,8 +36,12 @@ static inline enum ctx_state exception_enter(void)
return 0;
prev_ctx = this_cpu_read(context_tracking.state);
- if (prev_ctx != CONTEXT_KERNEL)
- context_tracking_exit(prev_ctx);
+ if (prev_ctx != CONTEXT_KERNEL) {
+ if (context_tracking_exit(prev_ctx)) {
+ if (cpu_isolated_strict())
+ cpu_isolated_exception();
+ }
+ }
return prev_ctx;
}
diff --git a/include/linux/cpu_isolated.h b/include/linux/cpu_isolated.h
index a3d17360f7ae..b0f1c2669b2f 100644
--- a/include/linux/cpu_isolated.h
+++ b/include/linux/cpu_isolated.h
@@ -15,10 +15,26 @@ static inline bool is_cpu_isolated(void)
}
extern void cpu_isolated_enter(void);
+extern void cpu_isolated_syscall(int nr);
+extern void cpu_isolated_exception(void);
extern void cpu_isolated_wait(void);
#else
static inline bool is_cpu_isolated(void) { return false; }
static inline void cpu_isolated_enter(void) { }
+static inline void cpu_isolated_syscall(int nr) { }
+static inline void cpu_isolated_exception(void) { }
#endif
+static inline bool cpu_isolated_strict(void)
+{
+#ifdef CONFIG_CPU_ISOLATED
+ if (tick_nohz_full_cpu(smp_processor_id()) &&
+ (current->cpu_isolated_flags &
+ (PR_CPU_ISOLATED_ENABLE | PR_CPU_ISOLATED_STRICT)) ==
+ (PR_CPU_ISOLATED_ENABLE | PR_CPU_ISOLATED_STRICT))
+ return true;
+#endif
+ return false;
+}
+
#endif
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index edb40b6b84db..0c11238a84fb 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -194,5 +194,6 @@ struct prctl_mm_map {
#define PR_SET_CPU_ISOLATED 47
#define PR_GET_CPU_ISOLATED 48
# define PR_CPU_ISOLATED_ENABLE (1 << 0)
+# define PR_CPU_ISOLATED_STRICT (1 << 1)
#endif /* _LINUX_PRCTL_H */
diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 36b6509c3e2a..c740850eea11 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -147,15 +147,16 @@ NOKPROBE_SYMBOL(context_tracking_user_enter);
* This call supports re-entrancy. This way it can be called from any exception
* handler without needing to know if we came from userspace or not.
*/
-void context_tracking_exit(enum ctx_state state)
+bool context_tracking_exit(enum ctx_state state)
{
unsigned long flags;
+ bool from_user = false;
if (!context_tracking_is_enabled())
- return;
+ return false;
if (in_interrupt())
- return;
+ return false;
local_irq_save(flags);
if (!context_tracking_recursion_enter())
@@ -169,6 +170,7 @@ void context_tracking_exit(enum ctx_state state)
*/
rcu_user_exit();
if (state == CONTEXT_USER) {
+ from_user = true;
vtime_user_exit(current);
trace_user_exit(0);
}
@@ -178,6 +180,7 @@ void context_tracking_exit(enum ctx_state state)
context_tracking_recursion_exit();
out_irq_restore:
local_irq_restore(flags);
+ return from_user;
}
NOKPROBE_SYMBOL(context_tracking_exit);
EXPORT_SYMBOL_GPL(context_tracking_exit);
diff --git a/kernel/time/cpu_isolated.c b/kernel/time/cpu_isolated.c
index e27259f30caf..d30bf3852897 100644
--- a/kernel/time/cpu_isolated.c
+++ b/kernel/time/cpu_isolated.c
@@ -10,6 +10,7 @@
#include <linux/swap.h>
#include <linux/vmstat.h>
#include <linux/cpu_isolated.h>
+#include <asm/unistd.h>
#include "tick-sched.h"
/*
@@ -69,3 +70,40 @@ void cpu_isolated_enter(void)
dump_stack();
}
}
+
+static void kill_cpu_isolated_strict_task(void)
+{
+ dump_stack();
+ current->cpu_isolated_flags &= ~PR_CPU_ISOLATED_ENABLE;
+ send_sig(SIGKILL, current, 1);
+}
+
+/*
+ * This routine is called from syscall entry (with the syscall number
+ * passed in) if the _STRICT flag is set.
+ */
+void cpu_isolated_syscall(int syscall)
+{
+ /* Ignore prctl() syscalls or any task exit. */
+ switch (syscall) {
+ case __NR_prctl:
+ case __NR_exit:
+ case __NR_exit_group:
+ return;
+ }
+
+ pr_warn("%s/%d: cpu_isolated strict mode violated by syscall %d\n",
+ current->comm, current->pid, syscall);
+ kill_cpu_isolated_strict_task();
+}
+
+/*
+ * This routine is called from any userspace exception if the _STRICT
+ * flag is set.
+ */
+void cpu_isolated_exception(void)
+{
+ pr_warn("%s/%d: cpu_isolated strict mode violated by exception\n",
+ current->comm, current->pid);
+ kill_cpu_isolated_strict_task();
+}
--
2.1.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox