* Re: [PATCH v3 4/7] dcache: allocate external names from reclaimable kmalloc caches
From: Mel Gorman @ 2018-07-19 8:42 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, linux-mm, linux-kernel, linux-api, Roman Gushchin,
Michal Hocko, Johannes Weiner, Christoph Lameter, David Rientjes,
Joonsoo Kim, Matthew Wilcox
In-Reply-To: <20180718133620.6205-5-vbabka@suse.cz>
On Wed, Jul 18, 2018 at 03:36:17PM +0200, Vlastimil Babka wrote:
> We can use the newly introduced kmalloc-reclaimable-X caches, to allocate
> external names in dcache, which will take care of the proper accounting
> automatically, and also improve anti-fragmentation page grouping.
>
> This effectively reverts commit f1782c9bc547 ("dcache: account external names
> as indirectly reclaimable memory") and instead passes __GFP_RECLAIMABLE to
> kmalloc(). The accounting thus moves from NR_INDIRECTLY_RECLAIMABLE_BYTES to
> NR_SLAB_RECLAIMABLE, which is also considered in MemAvailable calculation and
> overcommit decisions.
>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Mel Gorman <mgorman@techsingularity.net>
--
Mel Gorman
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3 3/7] mm, slab: allocate off-slab freelists as reclaimable when appropriate
From: Mel Gorman @ 2018-07-19 8:35 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, linux-mm, linux-kernel, linux-api, Roman Gushchin,
Michal Hocko, Johannes Weiner, Christoph Lameter, David Rientjes,
Joonsoo Kim, Matthew Wilcox
In-Reply-To: <20180718133620.6205-4-vbabka@suse.cz>
On Wed, Jul 18, 2018 at 03:36:16PM +0200, Vlastimil Babka wrote:
> In SLAB, OFF_SLAB caches allocate management structures (currently just the
> freelist) from kmalloc caches when placement in a slab page together with
> objects would lead to suboptimal memory usage. For SLAB_RECLAIM_ACCOUNT caches,
> we can allocate the freelists from the newly introduced reclaimable kmalloc
> caches, because shrinking the OFF_SLAB cache will in general result to freeing
> of the freelists as well. This should improve accounting and anti-fragmentation
> a bit.
>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
I'm not quite convinced by this one. The freelist cache is tied to the
lifetime of the slab and not the objects. A single freelist can be reclaimed
eventually but for caches with many objects per slab, it could take a lot
of shrinking random objects to reclaim one freelist. Functionally the
patch appears to be fine.
--
Mel Gorman
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3 2/7] mm, slab/slub: introduce kmalloc-reclaimable caches
From: Mel Gorman @ 2018-07-19 8:23 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, linux-mm, linux-kernel, linux-api, Roman Gushchin,
Michal Hocko, Johannes Weiner, Christoph Lameter, David Rientjes,
Joonsoo Kim, Matthew Wilcox
In-Reply-To: <20180718133620.6205-3-vbabka@suse.cz>
On Wed, Jul 18, 2018 at 03:36:15PM +0200, Vlastimil Babka wrote:
> Kmem caches can be created with a SLAB_RECLAIM_ACCOUNT flag, which indicates
> they contain objects which can be reclaimed under memory pressure (typically
> through a shrinker). This makes the slab pages accounted as NR_SLAB_RECLAIMABLE
> in vmstat, which is reflected also the MemAvailable meminfo counter and in
> overcommit decisions. The slab pages are also allocated with __GFP_RECLAIMABLE,
> which is good for anti-fragmentation through grouping pages by mobility.
>
> The generic kmalloc-X caches are created without this flag, but sometimes are
> used also for objects that can be reclaimed, which due to varying size cannot
> have a dedicated kmem cache with SLAB_RECLAIM_ACCOUNT flag. A prominent example
> are dcache external names, which prompted the creation of a new, manually
> managed vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES in commit f1782c9bc547
> ("dcache: account external names as indirectly reclaimable memory").
>
> To better handle this and any other similar cases, this patch introduces
> SLAB_RECLAIM_ACCOUNT variants of kmalloc caches, named kmalloc-rcl-X.
> They are used whenever the kmalloc() call passes __GFP_RECLAIMABLE among gfp
> flags. They are added to the kmalloc_caches array as a new type. Allocations
> with both __GFP_DMA and __GFP_RECLAIMABLE will use a dma type cache.
>
> This change only applies to SLAB and SLUB, not SLOB. This is fine, since SLOB's
> target are tiny system and this patch does add some overhead of kmem management
> objects.
>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>
> <SNIP>
>
> @@ -309,12 +310,19 @@ extern struct kmem_cache *kmalloc_caches[KMALLOC_TYPES][KMALLOC_SHIFT_HIGH + 1];
> static __always_inline unsigned int kmalloc_type(gfp_t flags)
> {
> int is_dma = 0;
> + int is_reclaimable;
>
> #ifdef CONFIG_ZONE_DMA
> is_dma = !!(flags & __GFP_DMA);
> #endif
>
> - return is_dma;
> + is_reclaimable = !!(flags & __GFP_RECLAIMABLE);
> +
> + /*
> + * If an allocation is botth __GFP_DMA and __GFP_RECLAIMABLE, return
> + * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
> + */
> + return (is_dma * 2) + (is_reclaimable & !is_dma);
> }
>
s/botth/both/
> /*
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 4614248ca381..614fb7ab8312 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1107,10 +1107,21 @@ void __init setup_kmalloc_cache_index_table(void)
> }
> }
>
> -static void __init new_kmalloc_cache(int idx, slab_flags_t flags)
> +static void __init
> +new_kmalloc_cache(int idx, int type, slab_flags_t flags)
> {
> - kmalloc_caches[KMALLOC_NORMAL][idx] = create_kmalloc_cache(
> - kmalloc_info[idx].name,
> + const char *name;
> +
> + if (type == KMALLOC_RECLAIM) {
> + flags |= SLAB_RECLAIM_ACCOUNT;
> + name = kasprintf(GFP_NOWAIT, "kmalloc-rcl-%u",
> + kmalloc_info[idx].size);
> + BUG_ON(!name);
> + } else {
> + name = kmalloc_info[idx].name;
> + }
> +
> + kmalloc_caches[type][idx] = create_kmalloc_cache(name,
> kmalloc_info[idx].size, flags, 0,
> kmalloc_info[idx].size);
> }
I was going to query that BUG_ON but if I'm reading it right, we just
have to be careful in the future that the "normal" kmalloc cache is always
initialised before the reclaimable cache or there will be issues.
> @@ -1122,22 +1133,25 @@ static void __init new_kmalloc_cache(int idx, slab_flags_t flags)
> */
> void __init create_kmalloc_caches(slab_flags_t flags)
> {
> - int i;
> - int type = KMALLOC_NORMAL;
> + int i, type;
>
> - for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
> - if (!kmalloc_caches[type][i])
> - new_kmalloc_cache(i, flags);
> + for (type = KMALLOC_NORMAL; type <= KMALLOC_RECLAIM; type++) {
> + for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
> + if (!kmalloc_caches[type][i])
> + new_kmalloc_cache(i, type, flags);
>
I don't see a problem here as such but the values of the KMALLOC_* types
is important both for this function and the kmalloc_type(). It might be
worth adding a warning that these functions be examined if updating the
types but then again, anyone trying and getting it wrong will have a
broken kernel so;
Acked-by: Mel Gorman <mgorman@techsingularity.net>
--
Mel Gorman
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3 1/7] mm, slab: combine kmalloc_caches and kmalloc_dma_caches
From: Mel Gorman @ 2018-07-19 8:10 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, linux-mm, linux-kernel, linux-api, Roman Gushchin,
Michal Hocko, Johannes Weiner, Christoph Lameter, David Rientjes,
Joonsoo Kim, Matthew Wilcox
In-Reply-To: <20180718133620.6205-2-vbabka@suse.cz>
On Wed, Jul 18, 2018 at 03:36:14PM +0200, Vlastimil Babka wrote:
> The kmalloc caches currently mainain separate (optional) array
> kmalloc_dma_caches for __GFP_DMA allocations. There are tests for __GFP_DMA in
> the allocation hotpaths. We can avoid the branches by combining kmalloc_caches
> and kmalloc_dma_caches into a single two-dimensional array where the outer
> dimension is cache "type". This will also allow to add kmalloc-reclaimable
> caches as a third type.
>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
I'm surprised there are so many kmalloc users that require the DMA zone.
Some of them are certainly bogus such as in drivers for archs that only
have one zone and is probably a reflection of the confusing naming. The
audit would be a mess and unrelated to the patch so for this patch;
Acked-by: Mel Gorman <mgorman@techsingularity.net>
--
Mel Gorman
SUSE Labs
^ permalink raw reply
* Re: [PATCH 24/32] vfs: syscall: Add fsopen() to prepare for superblock creation [ver #9]
From: Eric W. Biederman @ 2018-07-19 1:30 UTC (permalink / raw)
To: David Howells
Cc: Andy Lutomirski, Theodore Y. Ts'o, Linus Torvalds,
Andrew Lutomirski, Al Viro, Linux API, linux-fsdevel,
Linux Kernel Mailing List, Jann Horn
In-Reply-To: <3429.1531467024@warthog.procyon.org.uk>
David Howells <dhowells@redhat.com> writes:
> Andy Lutomirski <luto@amacapital.net> wrote:
>
>> > Also you can't currently directly create a bind mount from userspace as you
>> > can only bind from another path point - which you may not be able to access
>> > (either by permission failure or because it's not in your mount namespace).
>> >
>>
>> Are you trying to preserve the magic bind semantics with the new API?
>
> No, I'm pointing out that you can't emulate this by doing a bind mount from
> userspace if you can't access the thing you're binding from.
>
> Now, we could create a syscall that just picks up an extant superblock using a
> device and attaches it to a mount for you, but that would have to be at least
> partially parameterised - which would be very fs-dependent - so that it can
> know whether or not you're allowed to create another mount to that sb.
>
> What you're talking about is emulating sget() in userspace - when we have to
> do it in the kernel anyway if we still offer mount(2).
I am just going to chime in and say that it is absolutely a problem in
the current mount interface that when I mount a filesystem with fresh
parameters I don't know if it is generates an sget and a new super_block
or if it just increments the refcount on an existing super_block.
It is the kind of problem that is actually security sensitive and has
resulted in a security issue in the current linux kernel with respect to
proc.
So yes we absolutely need to have a clean way of dealing with:
mount /dev/sda3 /tmp
mount /dev/sda3 /mnt
So that the second one is forbidden fails. And userspace has to do the
equivalent of sget to get a file descriptor it can bind into the mount
namespace.
The deep problem is that the second mount does not parse the mount
options and userspace does not know that. So userspace thinks it is
getting one kind of mount and in practice it gets another (sometimes
with different security properties). Those different security
properties are an out and out bug. Although any kind of different and
unexpected properties can be a problem.
Eric
^ permalink raw reply
* Re: [RFC PATCH v2 16/27] mm: Modify can_follow_write_pte/pmd for shadow stack
From: Dave Hansen @ 2018-07-19 0:06 UTC (permalink / raw)
To: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra <pet>
In-Reply-To: <1531955428.12385.30.camel@intel.com>
>>> -static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)
>>> +static inline bool can_follow_write(pte_t pte, unsigned int flags,
>>> + struct vm_area_struct *vma)
>>> {
>>> - return pte_write(pte) ||
>>> - ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));
>>> + if (!is_shstk_mapping(vma->vm_flags)) {
>>> + if (pte_write(pte))
>>> + return true;
>> Let me see if I can say this another way.
>>
>> The bigger issue is that these patches change the semantics of
>> pte_write(). Before these patches, it meant that you *MUST* have this
>> bit set to write to the page controlled by the PTE. Now, it means: you
>> can write if this bit is set *OR* the shadowstack bit combination is set.
>
> Here, we only figure out (1) if the page is pointed by a writable PTE; or
> (2) if the page is pointed by a RO PTE (data or SHSTK) and it has been
> copied and it still exists. We are not trying to
> determine if the
> SHSTK PTE is writable (we know it is not).
Please think about the big picture. I'm not just talking about this
patch, but about every use of pte_write() in the kernel.
>> That's the fundamental problem. We need some code in the kernel that
>> logically represents the concept of "is this PTE a shadowstack PTE or a
>> PTE with the write bit set", and we will call that pte_write(), or maybe
>> pte_writable().
>>
>> You *have* to somehow rectify this situation. We can absolutely no
>> leave pte_write() in its current, ambiguous state where it has no real
>> meaning or where it is used to mean _both_ things depending on context.
>
> True, the processor can always write to a page through a shadow stack
> PTE, but it must do that with a CALL instruction. Can we define a
> write operation as: MOV r1, *(r2). Then we don't have any doubt on
> pte_write() any more.
No, we can't just move the target. :)
You can define it this way, but then you also need to go to every spot
in the kernel that calls pte_write() (and _PAGE_RW in fact) and audit it
to ensure it means "mov ..." and not push.
^ permalink raw reply
* Re: [RFC PATCH v2 16/27] mm: Modify can_follow_write_pte/pmd for shadow stack
From: Yu-cheng Yu @ 2018-07-18 23:10 UTC (permalink / raw)
To: Dave Hansen, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek
In-Reply-To: <3f158401-f0b6-7bf7-48ab-2958354b28ad@linux.intel.com>
On Wed, 2018-07-18 at 14:45 -0700, Dave Hansen wrote:
> On 07/18/2018 01:14 PM, Yu-cheng Yu wrote:
> >
> > On Tue, 2018-07-17 at 16:15 -0700, Dave Hansen wrote:
> > >
> > > On 07/17/2018 04:03 PM, Yu-cheng Yu wrote:
> > > >
> > > >
> > > > We need to find a way to differentiate "someone can write to this PTE"
> > > > from "the write bit is set in this PTE".
> > > Please think about this:
> > >
> > > Should pte_write() tell us whether PTE.W=1, or should it tell us
> > > that *something* can write to the PTE, which would include
> > > PTE.W=0/D=1?
> >
> > Is it better now?
> >
> >
> > Subject: [PATCH] mm: Modify can_follow_write_pte/pmd for shadow stack
> >
> > can_follow_write_pte/pmd look for the (RO & DIRTY) PTE/PMD to
> > verify a non-sharing RO page still exists after a broken COW.
> >
> > However, a shadow stack PTE is always RO & DIRTY; it can be:
> >
> > RO & DIRTY_HW - is_shstk_pte(pte) is true; or
> > RO & DIRTY_SW - the page is being shared.
> >
> > Update these functions to check a non-sharing shadow stack page
> > still exists after the COW.
> >
> > Also rename can_follow_write_pte/pmd() to can_follow_write() to
> > make their meaning clear; i.e. "Can we write to the page?", not
> > "Is the PTE writable?"
> >
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > ---
> > mm/gup.c | 38 ++++++++++++++++++++++++++++++++++----
> > mm/huge_memory.c | 19 ++++++++++++++-----
> > 2 files changed, 48 insertions(+), 9 deletions(-)
> >
> > diff --git a/mm/gup.c b/mm/gup.c
> > index fc5f98069f4e..316967996232 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -63,11 +63,41 @@ static int follow_pfn_pte(struct vm_area_struct *vma, unsigned long address,
> > /*
> > * FOLL_FORCE can write to even unwritable pte's, but only
> > * after we've gone through a COW cycle and they are dirty.
> > + *
> > + * Background:
> > + *
> > + * When we force-write to a read-only page, the page fault
> > + * handler copies the page and sets the new page's PTE to
> > + * RO & DIRTY. This routine tells
> > + *
> > + * "Can we write to the page?"
> > + *
> > + * by checking:
> > + *
> > + * (1) The page has been copied, i.e. FOLL_COW is set;
> > + * (2) The copy still exists and its PTE is RO & DIRTY.
> > + *
> > + * However, a shadow stack PTE is always RO & DIRTY; it can
> > + * be:
> > + *
> > + * RO & DIRTY_HW: when is_shstk_pte(pte) is true; or
> > + * RO & DIRTY_SW: when the page is being shared.
> > + *
> > + * To test a shadow stack's non-sharing page still exists,
> > + * we verify that the new page's PTE is_shstk_pte(pte).
> The content is getting there, but we need it next to the code, please.
>
> >
> > */
> > -static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)
> > +static inline bool can_follow_write(pte_t pte, unsigned int flags,
> > + struct vm_area_struct *vma)
> > {
> > - return pte_write(pte) ||
> > - ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));
> > + if (!is_shstk_mapping(vma->vm_flags)) {
> > + if (pte_write(pte))
> > + return true;
> Let me see if I can say this another way.
>
> The bigger issue is that these patches change the semantics of
> pte_write(). Before these patches, it meant that you *MUST* have this
> bit set to write to the page controlled by the PTE. Now, it means: you
> can write if this bit is set *OR* the shadowstack bit combination is set.
Here, we only figure out (1) if the page is pointed by a writable PTE; or
(2) if the page is pointed by a RO PTE (data or SHSTK) and it has been
copied and it still exists. We are not trying to
determine if the
SHSTK PTE is writable (we know it is not).
We look for the dirty bit to be sure the COW'ed page is still there.
The difference for the shadow stack case is that we look for the *hardware*
dirty bit. Perhaps we can create another macro, pte_ro_dirty_hw(), which
is equivalent to is_shstk_pte().
>
> That's the fundamental problem. We need some code in the kernel that
> logically represents the concept of "is this PTE a shadowstack PTE or a
> PTE with the write bit set", and we will call that pte_write(), or maybe
> pte_writable().
>
> You *have* to somehow rectify this situation. We can absolutely no
> leave pte_write() in its current, ambiguous state where it has no real
> meaning or where it is used to mean _both_ things depending on context.
True, the processor can always write to a page through a shadow stack
PTE, but it must do that with a CALL instruction. Can we define a
write operation as: MOV r1, *(r2). Then we don't have any doubt on
pte_write() any more.
>
> >
> > + return ((flags & FOLL_FORCE) && (flags & FOLL_COW) &&
> > + pte_dirty(pte));
> > + } else {
> > + return ((flags & FOLL_FORCE) && (flags & FOLL_COW) &&
> > + is_shstk_pte(pte));
> > + }
> > }
> Ok, it's rewrite time I guess.
>
> Yu-cheng, you may not know all the history, but this code is actually
> the source of the "Dirty COW" security issue. We need to be very, very
> careful with it, and super-explicit about all the logic. This is the
> time to blow up the comments and walk folks through exactly what we
> expect to happen.
>
> Anybody think I'm being too verbose? Is there a reason not to just go
> whole-hog on this sucker?
>
> static inline bool can_follow_write(pte_t pte, unsigned int flags,
> struct vm_area_struct *vma)
> {
> /*
> * FOLL_FORCE can "write" to hardware read-only PTEs, but
> * has to do a COW operation first. Do not allow the
> * hardware protection override unless we see FOLL_FORCE
> * *and* the COW has been performed by the fault code.
> */
> bool gup_cow_ok = (flags & FOLL_FORCE) &&
> (flags & FOLL_COW);
>
> /*
> * FOLL_COW flags tell us whether the page fault code did a COW
> * operation but not whether the PTE we are dealing with here
> * was COW'd. It could have been zapped and refaulted since the
> * COW operation.
> */
> bool pte_cow_ok;
>
> /* We have two COW pte "formats" */
> if (!is_shstk_mapping(vma->vm_flags)) {
> if (pte_write(pte)) {
> /* Any hardware-writable PTE is writable here */
> pte_cow_ok = true;
> } else {
> /* Is the COW-set dirty bit still there? */
> pte_cow_ok = pte_dirty(pte));
> }
> } else {
> /* Shadow stack PTEs are always hardware-writable */
>
> /*
> * Shadow stack pages do copy-on-access, so any present
> * shadow stack page has had a COW-equivalent performed.
> */
> pte_cow_ok = is_shstk_pte(pte));
> }
>
> return gup_cow_ok && pte_cow_ok;
> }
Ok, I will change it.
Yu-cheng
^ permalink raw reply
* Re: [RFC PATCH v2 16/27] mm: Modify can_follow_write_pte/pmd for shadow stack
From: Dave Hansen @ 2018-07-18 21:45 UTC (permalink / raw)
To: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra <pet>
In-Reply-To: <1531944882.10738.1.camel@intel.com>
On 07/18/2018 01:14 PM, Yu-cheng Yu wrote:
> On Tue, 2018-07-17 at 16:15 -0700, Dave Hansen wrote:
>> On 07/17/2018 04:03 PM, Yu-cheng Yu wrote:
>>>
>>> We need to find a way to differentiate "someone can write to this PTE"
>>> from "the write bit is set in this PTE".
>> Please think about this:
>>
>> Should pte_write() tell us whether PTE.W=1, or should it tell us
>> that *something* can write to the PTE, which would include
>> PTE.W=0/D=1?
>
>
> Is it better now?
>
>
> Subject: [PATCH] mm: Modify can_follow_write_pte/pmd for shadow stack
>
> can_follow_write_pte/pmd look for the (RO & DIRTY) PTE/PMD to
> verify a non-sharing RO page still exists after a broken COW.
>
> However, a shadow stack PTE is always RO & DIRTY; it can be:
>
> RO & DIRTY_HW - is_shstk_pte(pte) is true; or
> RO & DIRTY_SW - the page is being shared.
>
> Update these functions to check a non-sharing shadow stack page
> still exists after the COW.
>
> Also rename can_follow_write_pte/pmd() to can_follow_write() to
> make their meaning clear; i.e. "Can we write to the page?", not
> "Is the PTE writable?"
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
> mm/gup.c | 38 ++++++++++++++++++++++++++++++++++----
> mm/huge_memory.c | 19 ++++++++++++++-----
> 2 files changed, 48 insertions(+), 9 deletions(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index fc5f98069f4e..316967996232 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -63,11 +63,41 @@ static int follow_pfn_pte(struct vm_area_struct *vma, unsigned long address,
> /*
> * FOLL_FORCE can write to even unwritable pte's, but only
> * after we've gone through a COW cycle and they are dirty.
> + *
> + * Background:
> + *
> + * When we force-write to a read-only page, the page fault
> + * handler copies the page and sets the new page's PTE to
> + * RO & DIRTY. This routine tells
> + *
> + * "Can we write to the page?"
> + *
> + * by checking:
> + *
> + * (1) The page has been copied, i.e. FOLL_COW is set;
> + * (2) The copy still exists and its PTE is RO & DIRTY.
> + *
> + * However, a shadow stack PTE is always RO & DIRTY; it can
> + * be:
> + *
> + * RO & DIRTY_HW: when is_shstk_pte(pte) is true; or
> + * RO & DIRTY_SW: when the page is being shared.
> + *
> + * To test a shadow stack's non-sharing page still exists,
> + * we verify that the new page's PTE is_shstk_pte(pte).
The content is getting there, but we need it next to the code, please.
> */
> -static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)
> +static inline bool can_follow_write(pte_t pte, unsigned int flags,
> + struct vm_area_struct *vma)
> {
> - return pte_write(pte) ||
> - ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));
> + if (!is_shstk_mapping(vma->vm_flags)) {
> + if (pte_write(pte))
> + return true;
Let me see if I can say this another way.
The bigger issue is that these patches change the semantics of
pte_write(). Before these patches, it meant that you *MUST* have this
bit set to write to the page controlled by the PTE. Now, it means: you
can write if this bit is set *OR* the shadowstack bit combination is set.
That's the fundamental problem. We need some code in the kernel that
logically represents the concept of "is this PTE a shadowstack PTE or a
PTE with the write bit set", and we will call that pte_write(), or maybe
pte_writable().
You *have* to somehow rectify this situation. We can absolutely no
leave pte_write() in its current, ambiguous state where it has no real
meaning or where it is used to mean _both_ things depending on context.
> + return ((flags & FOLL_FORCE) && (flags & FOLL_COW) &&
> + pte_dirty(pte));
> + } else {
> + return ((flags & FOLL_FORCE) && (flags & FOLL_COW) &&
> + is_shstk_pte(pte));
> + }
> }
Ok, it's rewrite time I guess.
Yu-cheng, you may not know all the history, but this code is actually
the source of the "Dirty COW" security issue. We need to be very, very
careful with it, and super-explicit about all the logic. This is the
time to blow up the comments and walk folks through exactly what we
expect to happen.
Anybody think I'm being too verbose? Is there a reason not to just go
whole-hog on this sucker?
static inline bool can_follow_write(pte_t pte, unsigned int flags,
struct vm_area_struct *vma)
{
/*
* FOLL_FORCE can "write" to hardware read-only PTEs, but
* has to do a COW operation first. Do not allow the
* hardware protection override unless we see FOLL_FORCE
* *and* the COW has been performed by the fault code.
*/
bool gup_cow_ok = (flags & FOLL_FORCE) &&
(flags & FOLL_COW);
/*
* FOLL_COW flags tell us whether the page fault code did a COW
* operation but not whether the PTE we are dealing with here
* was COW'd. It could have been zapped and refaulted since the
* COW operation.
*/
bool pte_cow_ok;
/* We have two COW pte "formats" */
if (!is_shstk_mapping(vma->vm_flags)) {
if (pte_write(pte)) {
/* Any hardware-writable PTE is writable here */
pte_cow_ok = true;
} else {
/* Is the COW-set dirty bit still there? */
pte_cow_ok = pte_dirty(pte));
}
} else {
/* Shadow stack PTEs are always hardware-writable */
/*
* Shadow stack pages do copy-on-access, so any present
* shadow stack page has had a COW-equivalent performed.
*/
pte_cow_ok = is_shstk_pte(pte));
}
return gup_cow_ok && pte_cow_ok;
}
^ permalink raw reply
* Re: [RFC PATCH ghak90 (was ghak32) V3 10/10] rfkill: fix spelling mistake contidion to condition
From: Paul Moore @ 2018-07-18 20:56 UTC (permalink / raw)
To: rgb
Cc: cgroups, containers, linux-api, linux-audit, linux-fsdevel,
linux-kernel, netdev, ebiederm, luto, jlayton, carlos, dhowells,
viro, simo, Eric Paris, serge
In-Reply-To: <6118147ea27ab5831956cf9dcad15b241c791420.1528304204.git.rgb@redhat.com>
On Wed, Jun 6, 2018 at 1:02 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> net/rfkill/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
I would suggest splitting this patch from the audit container ID
patchset and sending it to the netdev folks. It really has nothing to
do with the audit container ID work, although I suspect I know why you
bumped into this spelling mistake ;)
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index 59d0eb9..e89a009 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -494,7 +494,7 @@ void rfkill_remove_epo_lock(void)
> /**
> * rfkill_is_epo_lock_active - returns true EPO is active
> *
> - * Returns 0 (false) if there is NOT an active EPO contidion,
> + * Returns 0 (false) if there is NOT an active EPO condition,
> * and 1 (true) if there is an active EPO contition, which
> * locks all radios in one of the BLOCKED states.
> *
> --
> 1.8.3.1
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH v2 02/17] y2038: Remove newstat family from default syscall set
From: Joseph Myers @ 2018-07-18 20:15 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Christoph Hellwig, Thomas Gleixner, y2038 Mailman List, Linux API,
linux-arch, GNU C Library, Albert ARIBAUD, Networking, Al Viro,
Peter Zijlstra, Darren Hart, Eric W . Biederman,
Dominik Brodowski
In-Reply-To: <CAK8P3a3a8Fyi3e24CE=SPmdEKskEjeDr9YZte7DU_jT2svF3Uw@mail.gmail.com>
On Tue, 17 Jul 2018, Arnd Bergmann wrote:
> That would definitely help. See below for the stat implementation
> I did in my musl libc prototype based on statx(). It passes the
> LTP syscall tests, but that doesn't mean all the corner cases
> are correct.
Well, you definitely need explicit timestamp conversions on the result of
statx to be usable in struct stat when userspace "long" is 64-bit, for BE
because otherwise the integer nanoseconds will be in the wrong place for
struct timespec, and for LE if the "__reserved is held in case we need a
yet finer resolution." might start being returned as nonzero (for integer
attoseconds, I suppose) in future.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply
* Re: [RFC PATCH v2 16/27] mm: Modify can_follow_write_pte/pmd for shadow stack
From: Yu-cheng Yu @ 2018-07-18 20:14 UTC (permalink / raw)
To: Dave Hansen, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek
In-Reply-To: <fa9db8c5-41c8-05e9-ad8d-dc6aaf11cb04@linux.intel.com>
On Tue, 2018-07-17 at 16:15 -0700, Dave Hansen wrote:
> On 07/17/2018 04:03 PM, Yu-cheng Yu wrote:
> >
> > We need to find a way to differentiate "someone can write to this PTE"
> > from "the write bit is set in this PTE".
> Please think about this:
>
> Should pte_write() tell us whether PTE.W=1, or should it tell us
> that *something* can write to the PTE, which would include
> PTE.W=0/D=1?
Is it better now?
Subject: [PATCH] mm: Modify can_follow_write_pte/pmd for shadow stack
can_follow_write_pte/pmd look for the (RO & DIRTY) PTE/PMD to
verify a non-sharing RO page still exists after a broken COW.
However, a shadow stack PTE is always RO & DIRTY; it can be:
RO & DIRTY_HW - is_shstk_pte(pte) is true; or
RO & DIRTY_SW - the page is being shared.
Update these functions to check a non-sharing shadow stack page
still exists after the COW.
Also rename can_follow_write_pte/pmd() to can_follow_write() to
make their meaning clear; i.e. "Can we write to the page?", not
"Is the PTE writable?"
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
---
mm/gup.c | 38 ++++++++++++++++++++++++++++++++++----
mm/huge_memory.c | 19 ++++++++++++++-----
2 files changed, 48 insertions(+), 9 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index fc5f98069f4e..316967996232 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -63,11 +63,41 @@ static int follow_pfn_pte(struct vm_area_struct *vma, unsigned long address,
/*
* FOLL_FORCE can write to even unwritable pte's, but only
* after we've gone through a COW cycle and they are dirty.
+ *
+ * Background:
+ *
+ * When we force-write to a read-only page, the page fault
+ * handler copies the page and sets the new page's PTE to
+ * RO & DIRTY. This routine tells
+ *
+ * "Can we write to the page?"
+ *
+ * by checking:
+ *
+ * (1) The page has been copied, i.e. FOLL_COW is set;
+ * (2) The copy still exists and its PTE is RO & DIRTY.
+ *
+ * However, a shadow stack PTE is always RO & DIRTY; it can
+ * be:
+ *
+ * RO & DIRTY_HW: when is_shstk_pte(pte) is true; or
+ * RO & DIRTY_SW: when the page is being shared.
+ *
+ * To test a shadow stack's non-sharing page still exists,
+ * we verify that the new page's PTE is_shstk_pte(pte).
*/
-static inline bool can_follow_write_pte(pte_t pte, unsigned int flags)
+static inline bool can_follow_write(pte_t pte, unsigned int flags,
+ struct vm_area_struct *vma)
{
- return pte_write(pte) ||
- ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pte_dirty(pte));
+ if (!is_shstk_mapping(vma->vm_flags)) {
+ if (pte_write(pte))
+ return true;
+ return ((flags & FOLL_FORCE) && (flags & FOLL_COW) &&
+ pte_dirty(pte));
+ } else {
+ return ((flags & FOLL_FORCE) && (flags & FOLL_COW) &&
+ is_shstk_pte(pte));
+ }
}
static struct page *follow_page_pte(struct vm_area_struct *vma,
@@ -105,7 +135,7 @@ static struct page *follow_page_pte(struct vm_area_struct *vma,
}
if ((flags & FOLL_NUMA) && pte_protnone(pte))
goto no_page;
- if ((flags & FOLL_WRITE) && !can_follow_write_pte(pte, flags)) {
+ if ((flags & FOLL_WRITE) && !can_follow_write(pte, flags, vma)) {
pte_unmap_unlock(ptep, ptl);
return NULL;
}
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 7f3e11d3b64a..822a563678b5 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1388,11 +1388,20 @@ int do_huge_pmd_wp_page(struct vm_fault *vmf, pmd_t orig_pmd)
/*
* FOLL_FORCE can write to even unwritable pmd's, but only
* after we've gone through a COW cycle and they are dirty.
+ * See comments in mm/gup.c, can_follow_write().
*/
-static inline bool can_follow_write_pmd(pmd_t pmd, unsigned int flags)
-{
- return pmd_write(pmd) ||
- ((flags & FOLL_FORCE) && (flags & FOLL_COW) && pmd_dirty(pmd));
+static inline bool can_follow_write(pmd_t pmd, unsigned int flags,
+ struct vm_area_struct *vma)
+{
+ if (!is_shstk_mapping(vma->vm_flags)) {
+ if (pmd_write(pmd))
+ return true;
+ return ((flags & FOLL_FORCE) && (flags & FOLL_COW) &&
+ pmd_dirty(pmd));
+ } else {
+ return ((flags & FOLL_FORCE) && (flags & FOLL_COW) &&
+ is_shstk_pmd(pmd));
+ }
}
struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
@@ -1405,7 +1414,7 @@ struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
assert_spin_locked(pmd_lockptr(mm, pmd));
- if (flags & FOLL_WRITE && !can_follow_write_pmd(*pmd, flags))
+ if (flags & FOLL_WRITE && !can_follow_write(*pmd, flags, vma))
goto out;
/* Avoid dumping huge zero page */
--
^ permalink raw reply related
* Re: [PATCHSET v3] block: Separating discards from writes in Linux IO statistics
From: Jens Axboe @ 2018-07-18 14:45 UTC (permalink / raw)
To: Tejun Heo
Cc: michaelcallahan, newella, linux-block, linux-kernel, kernel-team,
linux-api
In-Reply-To: <20180718114741.2580313-1-tj@kernel.org>
On 7/18/18 5:47 AM, Tejun Heo wrote:
> Hello,
>
> Changes from v2: Refreshed on top of for-4.19/block.
Applied for 4.19, thanks Tejun.
--
Jens Axboe
^ permalink raw reply
* [PATCH v3 7/7] mm, slab: shorten kmalloc cache names for large sizes
From: Vlastimil Babka @ 2018-07-18 13:36 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, linux-api, Roman Gushchin, Michal Hocko,
Johannes Weiner, Christoph Lameter, David Rientjes, Joonsoo Kim,
Mel Gorman, Matthew Wilcox, Vlastimil Babka
In-Reply-To: <20180718133620.6205-1-vbabka@suse.cz>
Kmalloc cache names can get quite long for large object sizes, when the sizes
are expressed in bytes. Use 'k' and 'M' prefixes to make the names as short
as possible e.g. in /proc/slabinfo. This works, as we mostly use power-of-two
sizes, with exceptions only below 1k.
Example: 'kmalloc-4194304' becomes 'kmalloc-4M'
Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/slab_common.c | 38 ++++++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 614fb7ab8312..04d71ead7d12 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1049,15 +1049,15 @@ const struct kmalloc_info_struct kmalloc_info[] __initconst = {
{"kmalloc-16", 16}, {"kmalloc-32", 32},
{"kmalloc-64", 64}, {"kmalloc-128", 128},
{"kmalloc-256", 256}, {"kmalloc-512", 512},
- {"kmalloc-1024", 1024}, {"kmalloc-2048", 2048},
- {"kmalloc-4096", 4096}, {"kmalloc-8192", 8192},
- {"kmalloc-16384", 16384}, {"kmalloc-32768", 32768},
- {"kmalloc-65536", 65536}, {"kmalloc-131072", 131072},
- {"kmalloc-262144", 262144}, {"kmalloc-524288", 524288},
- {"kmalloc-1048576", 1048576}, {"kmalloc-2097152", 2097152},
- {"kmalloc-4194304", 4194304}, {"kmalloc-8388608", 8388608},
- {"kmalloc-16777216", 16777216}, {"kmalloc-33554432", 33554432},
- {"kmalloc-67108864", 67108864}
+ {"kmalloc-1k", 1024}, {"kmalloc-2k", 2048},
+ {"kmalloc-4k", 4096}, {"kmalloc-8k", 8192},
+ {"kmalloc-16k", 16384}, {"kmalloc-32k", 32768},
+ {"kmalloc-64k", 65536}, {"kmalloc-128k", 131072},
+ {"kmalloc-256k", 262144}, {"kmalloc-512k", 524288},
+ {"kmalloc-1M", 1048576}, {"kmalloc-2M", 2097152},
+ {"kmalloc-4M", 4194304}, {"kmalloc-8M", 8388608},
+ {"kmalloc-16M", 16777216}, {"kmalloc-32M", 33554432},
+ {"kmalloc-64M", 67108864}
};
/*
@@ -1107,6 +1107,21 @@ void __init setup_kmalloc_cache_index_table(void)
}
}
+static const char *
+kmalloc_cache_name(const char *prefix, unsigned int size)
+{
+
+ static const char units[3] = "\0kM";
+ int idx = 0;
+
+ while (size >= 1024 && (size % 1024 == 0)) {
+ size /= 1024;
+ idx++;
+ }
+
+ return kasprintf(GFP_NOWAIT, "%s-%u%c", prefix, size, units[idx]);
+}
+
static void __init
new_kmalloc_cache(int idx, int type, slab_flags_t flags)
{
@@ -1114,7 +1129,7 @@ new_kmalloc_cache(int idx, int type, slab_flags_t flags)
if (type == KMALLOC_RECLAIM) {
flags |= SLAB_RECLAIM_ACCOUNT;
- name = kasprintf(GFP_NOWAIT, "kmalloc-rcl-%u",
+ name = kmalloc_cache_name("kmalloc-rcl",
kmalloc_info[idx].size);
BUG_ON(!name);
} else {
@@ -1163,8 +1178,7 @@ void __init create_kmalloc_caches(slab_flags_t flags)
if (s) {
unsigned int size = kmalloc_size(i);
- char *n = kasprintf(GFP_NOWAIT,
- "dma-kmalloc-%u", size);
+ const char *n = kmalloc_cache_name("dma-kmalloc", size);
BUG_ON(!n);
kmalloc_caches[KMALLOC_DMA][i] = create_kmalloc_cache(
--
2.18.0
^ permalink raw reply related
* [PATCH v3 6/7] mm, proc: add KReclaimable to /proc/meminfo
From: Vlastimil Babka @ 2018-07-18 13:36 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, linux-api, Roman Gushchin, Michal Hocko,
Johannes Weiner, Christoph Lameter, David Rientjes, Joonsoo Kim,
Mel Gorman, Matthew Wilcox, Vlastimil Babka
In-Reply-To: <20180718133620.6205-1-vbabka@suse.cz>
The vmstat NR_KERNEL_MISC_RECLAIMABLE counter is for kernel non-slab
allocations that can be reclaimed via shrinker. In /proc/meminfo, we can show
the sum of all reclaimable kernel allocations (including slab) as
"KReclaimable". Add the same counter also to per-node meminfo under /sys
With this counter, users will have more complete information about
kernel memory usage. Non-slab reclaimable pages (currently just the ION
allocator) will not be missing from /proc/meminfo, making users wonder
where part of their memory went. More precisely, they already appear in
MemAvailable, but without the new counter, it's not obvious why the
value in MemAvailable doesn't fully correspond with the sum of other
counters participating in it.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
Documentation/filesystems/proc.txt | 4 ++++
drivers/base/node.c | 19 ++++++++++++-------
fs/proc/meminfo.c | 16 ++++++++--------
3 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index 520f6a84cf50..6a255f960ab5 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -858,6 +858,7 @@ Writeback: 0 kB
AnonPages: 861800 kB
Mapped: 280372 kB
Shmem: 644 kB
+KReclaimable: 168048 kB
Slab: 284364 kB
SReclaimable: 159856 kB
SUnreclaim: 124508 kB
@@ -921,6 +922,9 @@ AnonHugePages: Non-file backed huge pages mapped into userspace page tables
ShmemHugePages: Memory used by shared memory (shmem) and tmpfs allocated
with huge pages
ShmemPmdMapped: Shared memory mapped into userspace with huge pages
+KReclaimable: Kernel allocations that the kernel will attempt to reclaim
+ under memory pressure. Includes SReclaimable (below), and other
+ direct allocations with a shrinker.
Slab: in-kernel data structures cache
SReclaimable: Part of Slab, that might be reclaimed, such as caches
SUnreclaim: Part of Slab, that cannot be reclaimed on memory pressure
diff --git a/drivers/base/node.c b/drivers/base/node.c
index a5e821d09656..81cef8031eae 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -67,8 +67,11 @@ static ssize_t node_read_meminfo(struct device *dev,
int nid = dev->id;
struct pglist_data *pgdat = NODE_DATA(nid);
struct sysinfo i;
+ unsigned long sreclaimable, sunreclaimable;
si_meminfo_node(&i, nid);
+ sreclaimable = node_page_state(pgdat, NR_SLAB_RECLAIMABLE);
+ sunreclaimable = node_page_state(pgdat, NR_SLAB_UNRECLAIMABLE);
n = sprintf(buf,
"Node %d MemTotal: %8lu kB\n"
"Node %d MemFree: %8lu kB\n"
@@ -118,6 +121,7 @@ static ssize_t node_read_meminfo(struct device *dev,
"Node %d NFS_Unstable: %8lu kB\n"
"Node %d Bounce: %8lu kB\n"
"Node %d WritebackTmp: %8lu kB\n"
+ "Node %d KReclaimable: %8lu kB\n"
"Node %d Slab: %8lu kB\n"
"Node %d SReclaimable: %8lu kB\n"
"Node %d SUnreclaim: %8lu kB\n"
@@ -138,20 +142,21 @@ static ssize_t node_read_meminfo(struct device *dev,
nid, K(node_page_state(pgdat, NR_UNSTABLE_NFS)),
nid, K(sum_zone_node_page_state(nid, NR_BOUNCE)),
nid, K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
- nid, K(node_page_state(pgdat, NR_SLAB_RECLAIMABLE) +
- node_page_state(pgdat, NR_SLAB_UNRECLAIMABLE)),
- nid, K(node_page_state(pgdat, NR_SLAB_RECLAIMABLE)),
+ nid, K(sreclaimable +
+ node_page_state(pgdat, NR_KERNEL_MISC_RECLAIMABLE)),
+ nid, K(sreclaimable + sunreclaimable),
+ nid, K(sreclaimable),
+ nid, K(sunreclaimable)
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
- nid, K(node_page_state(pgdat, NR_SLAB_UNRECLAIMABLE)),
+ ,
nid, K(node_page_state(pgdat, NR_ANON_THPS) *
HPAGE_PMD_NR),
nid, K(node_page_state(pgdat, NR_SHMEM_THPS) *
HPAGE_PMD_NR),
nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) *
- HPAGE_PMD_NR));
-#else
- nid, K(node_page_state(pgdat, NR_SLAB_UNRECLAIMABLE)));
+ HPAGE_PMD_NR)
#endif
+ );
n += hugetlb_report_node_meminfo(nid, buf + n);
return n;
}
diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
index 2fb04846ed11..61a18477bc07 100644
--- a/fs/proc/meminfo.c
+++ b/fs/proc/meminfo.c
@@ -37,6 +37,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
long cached;
long available;
unsigned long pages[NR_LRU_LISTS];
+ unsigned long sreclaimable, sunreclaim;
int lru;
si_meminfo(&i);
@@ -52,6 +53,8 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
pages[lru] = global_node_page_state(NR_LRU_BASE + lru);
available = si_mem_available();
+ sreclaimable = global_node_page_state(NR_SLAB_RECLAIMABLE);
+ sunreclaim = global_node_page_state(NR_SLAB_UNRECLAIMABLE);
show_val_kb(m, "MemTotal: ", i.totalram);
show_val_kb(m, "MemFree: ", i.freeram);
@@ -93,14 +96,11 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
show_val_kb(m, "Mapped: ",
global_node_page_state(NR_FILE_MAPPED));
show_val_kb(m, "Shmem: ", i.sharedram);
- show_val_kb(m, "Slab: ",
- global_node_page_state(NR_SLAB_RECLAIMABLE) +
- global_node_page_state(NR_SLAB_UNRECLAIMABLE));
-
- show_val_kb(m, "SReclaimable: ",
- global_node_page_state(NR_SLAB_RECLAIMABLE));
- show_val_kb(m, "SUnreclaim: ",
- global_node_page_state(NR_SLAB_UNRECLAIMABLE));
+ show_val_kb(m, "KReclaimable: ", sreclaimable +
+ global_node_page_state(NR_KERNEL_MISC_RECLAIMABLE));
+ show_val_kb(m, "Slab: ", sreclaimable + sunreclaim);
+ show_val_kb(m, "SReclaimable: ", sreclaimable);
+ show_val_kb(m, "SUnreclaim: ", sunreclaim);
seq_printf(m, "KernelStack: %8lu kB\n",
global_zone_page_state(NR_KERNEL_STACK_KB));
show_val_kb(m, "PageTables: ",
--
2.18.0
^ permalink raw reply related
* [PATCH v3 5/7] mm: rename and change semantics of nr_indirectly_reclaimable_bytes
From: Vlastimil Babka @ 2018-07-18 13:36 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, linux-api, Roman Gushchin, Michal Hocko,
Johannes Weiner, Christoph Lameter, David Rientjes, Joonsoo Kim,
Mel Gorman, Matthew Wilcox, Vlastimil Babka, Vijayanand Jitta,
Laura Abbott, Sumit Semwal
In-Reply-To: <20180718133620.6205-1-vbabka@suse.cz>
The vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES was introduced by commit
eb59254608bc ("mm: introduce NR_INDIRECTLY_RECLAIMABLE_BYTES") with the goal of
accounting objects that can be reclaimed, but cannot be allocated via a
SLAB_RECLAIM_ACCOUNT cache. This is now possible via kmalloc() with
__GFP_RECLAIMABLE flag, and the dcache external names user is converted.
The counter is however still useful for accounting direct page allocations
(i.e. not slab) with a shrinker, such as the ION page pool. So keep it, and:
- change granularity to pages to be more like other counters; sub-page
allocations should be able to use kmalloc
- rename the counter to NR_KERNEL_MISC_RECLAIMABLE
- expose the counter again in vmstat as "nr_kernel_misc_reclaimable"; we can
again remove the check for not printing "hidden" counters
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Vijayanand Jitta <vjitta@codeaurora.org>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
---
drivers/staging/android/ion/ion_page_pool.c | 8 ++++----
include/linux/mmzone.h | 2 +-
mm/page_alloc.c | 19 +++++++------------
mm/util.c | 3 +--
mm/vmstat.c | 6 +-----
5 files changed, 14 insertions(+), 24 deletions(-)
diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c
index 9bc56eb48d2a..0d2a95957ee8 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -33,8 +33,8 @@ static void ion_page_pool_add(struct ion_page_pool *pool, struct page *page)
pool->low_count++;
}
- mod_node_page_state(page_pgdat(page), NR_INDIRECTLY_RECLAIMABLE_BYTES,
- (1 << (PAGE_SHIFT + pool->order)));
+ mod_node_page_state(page_pgdat(page), NR_KERNEL_MISC_RECLAIMABLE,
+ 1 << pool->order);
mutex_unlock(&pool->mutex);
}
@@ -53,8 +53,8 @@ static struct page *ion_page_pool_remove(struct ion_page_pool *pool, bool high)
}
list_del(&page->lru);
- mod_node_page_state(page_pgdat(page), NR_INDIRECTLY_RECLAIMABLE_BYTES,
- -(1 << (PAGE_SHIFT + pool->order)));
+ mod_node_page_state(page_pgdat(page), NR_KERNEL_MISC_RECLAIMABLE,
+ -(1 << pool->order));
return page;
}
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 32699b2dc52a..c2f6bc4c9e8a 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -180,7 +180,7 @@ enum node_stat_item {
NR_VMSCAN_IMMEDIATE, /* Prioritise for reclaim when writeback ends */
NR_DIRTIED, /* page dirtyings since bootup */
NR_WRITTEN, /* page writings since bootup */
- NR_INDIRECTLY_RECLAIMABLE_BYTES, /* measured in bytes */
+ NR_KERNEL_MISC_RECLAIMABLE, /* reclaimable non-slab kernel pages */
NR_VM_NODE_STAT_ITEMS
};
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5d800d61ddb7..91f75bf4404d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4704,6 +4704,7 @@ long si_mem_available(void)
unsigned long pagecache;
unsigned long wmark_low = 0;
unsigned long pages[NR_LRU_LISTS];
+ unsigned long reclaimable;
struct zone *zone;
int lru;
@@ -4729,19 +4730,13 @@ long si_mem_available(void)
available += pagecache;
/*
- * Part of the reclaimable slab consists of items that are in use,
- * and cannot be freed. Cap this estimate at the low watermark.
+ * Part of the reclaimable slab and other kernel memory consists of
+ * items that are in use, and cannot be freed. Cap this estimate at the
+ * low watermark.
*/
- available += global_node_page_state(NR_SLAB_RECLAIMABLE) -
- min(global_node_page_state(NR_SLAB_RECLAIMABLE) / 2,
- wmark_low);
-
- /*
- * Part of the kernel memory, which can be released under memory
- * pressure.
- */
- available += global_node_page_state(NR_INDIRECTLY_RECLAIMABLE_BYTES) >>
- PAGE_SHIFT;
+ reclaimable = global_node_page_state(NR_SLAB_RECLAIMABLE) +
+ global_node_page_state(NR_KERNEL_MISC_RECLAIMABLE);
+ available += reclaimable - min(reclaimable / 2, wmark_low);
if (available < 0)
available = 0;
diff --git a/mm/util.c b/mm/util.c
index 3351659200e6..891f0654e7b5 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -675,8 +675,7 @@ int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin)
* Part of the kernel memory, which can be released
* under memory pressure.
*/
- free += global_node_page_state(
- NR_INDIRECTLY_RECLAIMABLE_BYTES) >> PAGE_SHIFT;
+ free += global_node_page_state(NR_KERNEL_MISC_RECLAIMABLE);
/*
* Leave reserved pages. The pages are not for anonymous pages.
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 8ba0870ecddd..c5e52f94ba5f 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1161,7 +1161,7 @@ const char * const vmstat_text[] = {
"nr_vmscan_immediate_reclaim",
"nr_dirtied",
"nr_written",
- "", /* nr_indirectly_reclaimable */
+ "nr_kernel_misc_reclaimable",
/* enum writeback_stat_item counters */
"nr_dirty_threshold",
@@ -1704,10 +1704,6 @@ static int vmstat_show(struct seq_file *m, void *arg)
unsigned long *l = arg;
unsigned long off = l - (unsigned long *)m->private;
- /* Skip hidden vmstat items. */
- if (*vmstat_text[off] == '\0')
- return 0;
-
seq_puts(m, vmstat_text[off]);
seq_put_decimal_ull(m, " ", *l);
seq_putc(m, '\n');
--
2.18.0
^ permalink raw reply related
* [PATCH v3 4/7] dcache: allocate external names from reclaimable kmalloc caches
From: Vlastimil Babka @ 2018-07-18 13:36 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, linux-api, Roman Gushchin, Michal Hocko,
Johannes Weiner, Christoph Lameter, David Rientjes, Joonsoo Kim,
Mel Gorman, Matthew Wilcox, Vlastimil Babka
In-Reply-To: <20180718133620.6205-1-vbabka@suse.cz>
We can use the newly introduced kmalloc-reclaimable-X caches, to allocate
external names in dcache, which will take care of the proper accounting
automatically, and also improve anti-fragmentation page grouping.
This effectively reverts commit f1782c9bc547 ("dcache: account external names
as indirectly reclaimable memory") and instead passes __GFP_RECLAIMABLE to
kmalloc(). The accounting thus moves from NR_INDIRECTLY_RECLAIMABLE_BYTES to
NR_SLAB_RECLAIMABLE, which is also considered in MemAvailable calculation and
overcommit decisions.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
fs/dcache.c | 38 +++++++++-----------------------------
1 file changed, 9 insertions(+), 29 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 0e8e5de3c48a..518c9ed8db8c 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -257,24 +257,10 @@ static void __d_free(struct rcu_head *head)
kmem_cache_free(dentry_cache, dentry);
}
-static void __d_free_external_name(struct rcu_head *head)
-{
- struct external_name *name = container_of(head, struct external_name,
- u.head);
-
- mod_node_page_state(page_pgdat(virt_to_page(name)),
- NR_INDIRECTLY_RECLAIMABLE_BYTES,
- -ksize(name));
-
- kfree(name);
-}
-
static void __d_free_external(struct rcu_head *head)
{
struct dentry *dentry = container_of(head, struct dentry, d_u.d_rcu);
-
- __d_free_external_name(&external_name(dentry)->u.head);
-
+ kfree(external_name(dentry));
kmem_cache_free(dentry_cache, dentry);
}
@@ -305,7 +291,7 @@ void release_dentry_name_snapshot(struct name_snapshot *name)
struct external_name *p;
p = container_of(name->name, struct external_name, name[0]);
if (unlikely(atomic_dec_and_test(&p->u.count)))
- call_rcu(&p->u.head, __d_free_external_name);
+ kfree_rcu(p, u.head);
}
}
EXPORT_SYMBOL(release_dentry_name_snapshot);
@@ -1608,7 +1594,6 @@ EXPORT_SYMBOL(d_invalidate);
struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
{
- struct external_name *ext = NULL;
struct dentry *dentry;
char *dname;
int err;
@@ -1629,14 +1614,15 @@ struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
dname = dentry->d_iname;
} else if (name->len > DNAME_INLINE_LEN-1) {
size_t size = offsetof(struct external_name, name[1]);
-
- ext = kmalloc(size + name->len, GFP_KERNEL_ACCOUNT);
- if (!ext) {
+ struct external_name *p = kmalloc(size + name->len,
+ GFP_KERNEL_ACCOUNT |
+ __GFP_RECLAIMABLE);
+ if (!p) {
kmem_cache_free(dentry_cache, dentry);
return NULL;
}
- atomic_set(&ext->u.count, 1);
- dname = ext->name;
+ atomic_set(&p->u.count, 1);
+ dname = p->name;
} else {
dname = dentry->d_iname;
}
@@ -1675,12 +1661,6 @@ struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
}
}
- if (unlikely(ext)) {
- pg_data_t *pgdat = page_pgdat(virt_to_page(ext));
- mod_node_page_state(pgdat, NR_INDIRECTLY_RECLAIMABLE_BYTES,
- ksize(ext));
- }
-
this_cpu_inc(nr_dentry);
return dentry;
@@ -2761,7 +2741,7 @@ static void copy_name(struct dentry *dentry, struct dentry *target)
dentry->d_name.hash_len = target->d_name.hash_len;
}
if (old_name && likely(atomic_dec_and_test(&old_name->u.count)))
- call_rcu(&old_name->u.head, __d_free_external_name);
+ kfree_rcu(old_name, u.head);
}
/*
--
2.18.0
^ permalink raw reply related
* [PATCH v3 3/7] mm, slab: allocate off-slab freelists as reclaimable when appropriate
From: Vlastimil Babka @ 2018-07-18 13:36 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, linux-api, Roman Gushchin, Michal Hocko,
Johannes Weiner, Christoph Lameter, David Rientjes, Joonsoo Kim,
Mel Gorman, Matthew Wilcox, Vlastimil Babka
In-Reply-To: <20180718133620.6205-1-vbabka@suse.cz>
In SLAB, OFF_SLAB caches allocate management structures (currently just the
freelist) from kmalloc caches when placement in a slab page together with
objects would lead to suboptimal memory usage. For SLAB_RECLAIM_ACCOUNT caches,
we can allocate the freelists from the newly introduced reclaimable kmalloc
caches, because shrinking the OFF_SLAB cache will in general result to freeing
of the freelists as well. This should improve accounting and anti-fragmentation
a bit.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/slab.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/slab.c b/mm/slab.c
index 9515798f37b2..99d779ba2b92 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2140,8 +2140,13 @@ int __kmem_cache_create(struct kmem_cache *cachep, slab_flags_t flags)
#endif
if (OFF_SLAB(cachep)) {
+ /*
+ * If this cache is reclaimable, allocate also freelists from
+ * a reclaimable kmalloc cache.
+ */
cachep->freelist_cache =
- kmalloc_slab(cachep->freelist_size, 0u);
+ kmalloc_slab(cachep->freelist_size,
+ cachep->allocflags & __GFP_RECLAIMABLE);
}
err = setup_cpu_cache(cachep, gfp);
--
2.18.0
^ permalink raw reply related
* [PATCH v3 2/7] mm, slab/slub: introduce kmalloc-reclaimable caches
From: Vlastimil Babka @ 2018-07-18 13:36 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, linux-api, Roman Gushchin, Michal Hocko,
Johannes Weiner, Christoph Lameter, David Rientjes, Joonsoo Kim,
Mel Gorman, Matthew Wilcox, Vlastimil Babka
In-Reply-To: <20180718133620.6205-1-vbabka@suse.cz>
Kmem caches can be created with a SLAB_RECLAIM_ACCOUNT flag, which indicates
they contain objects which can be reclaimed under memory pressure (typically
through a shrinker). This makes the slab pages accounted as NR_SLAB_RECLAIMABLE
in vmstat, which is reflected also the MemAvailable meminfo counter and in
overcommit decisions. The slab pages are also allocated with __GFP_RECLAIMABLE,
which is good for anti-fragmentation through grouping pages by mobility.
The generic kmalloc-X caches are created without this flag, but sometimes are
used also for objects that can be reclaimed, which due to varying size cannot
have a dedicated kmem cache with SLAB_RECLAIM_ACCOUNT flag. A prominent example
are dcache external names, which prompted the creation of a new, manually
managed vmstat counter NR_INDIRECTLY_RECLAIMABLE_BYTES in commit f1782c9bc547
("dcache: account external names as indirectly reclaimable memory").
To better handle this and any other similar cases, this patch introduces
SLAB_RECLAIM_ACCOUNT variants of kmalloc caches, named kmalloc-rcl-X.
They are used whenever the kmalloc() call passes __GFP_RECLAIMABLE among gfp
flags. They are added to the kmalloc_caches array as a new type. Allocations
with both __GFP_DMA and __GFP_RECLAIMABLE will use a dma type cache.
This change only applies to SLAB and SLUB, not SLOB. This is fine, since SLOB's
target are tiny system and this patch does add some overhead of kmem management
objects.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
include/linux/slab.h | 16 +++++++++++----
mm/slab_common.c | 48 ++++++++++++++++++++++++++++----------------
2 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 4299c59353a1..d89e934e0d8b 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -296,11 +296,12 @@ static inline void __check_heap_object(const void *ptr, unsigned long n,
(KMALLOC_MIN_SIZE) : 16)
#define KMALLOC_NORMAL 0
+#define KMALLOC_RECLAIM 1
#ifdef CONFIG_ZONE_DMA
-#define KMALLOC_DMA 1
-#define KMALLOC_TYPES 2
+#define KMALLOC_DMA 2
+#define KMALLOC_TYPES 3
#else
-#define KMALLOC_TYPES 1
+#define KMALLOC_TYPES 2
#endif
#ifndef CONFIG_SLOB
@@ -309,12 +310,19 @@ extern struct kmem_cache *kmalloc_caches[KMALLOC_TYPES][KMALLOC_SHIFT_HIGH + 1];
static __always_inline unsigned int kmalloc_type(gfp_t flags)
{
int is_dma = 0;
+ int is_reclaimable;
#ifdef CONFIG_ZONE_DMA
is_dma = !!(flags & __GFP_DMA);
#endif
- return is_dma;
+ is_reclaimable = !!(flags & __GFP_RECLAIMABLE);
+
+ /*
+ * If an allocation is botth __GFP_DMA and __GFP_RECLAIMABLE, return
+ * KMALLOC_DMA and effectively ignore __GFP_RECLAIMABLE
+ */
+ return (is_dma * 2) + (is_reclaimable & !is_dma);
}
/*
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 4614248ca381..614fb7ab8312 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1107,10 +1107,21 @@ void __init setup_kmalloc_cache_index_table(void)
}
}
-static void __init new_kmalloc_cache(int idx, slab_flags_t flags)
+static void __init
+new_kmalloc_cache(int idx, int type, slab_flags_t flags)
{
- kmalloc_caches[KMALLOC_NORMAL][idx] = create_kmalloc_cache(
- kmalloc_info[idx].name,
+ const char *name;
+
+ if (type == KMALLOC_RECLAIM) {
+ flags |= SLAB_RECLAIM_ACCOUNT;
+ name = kasprintf(GFP_NOWAIT, "kmalloc-rcl-%u",
+ kmalloc_info[idx].size);
+ BUG_ON(!name);
+ } else {
+ name = kmalloc_info[idx].name;
+ }
+
+ kmalloc_caches[type][idx] = create_kmalloc_cache(name,
kmalloc_info[idx].size, flags, 0,
kmalloc_info[idx].size);
}
@@ -1122,22 +1133,25 @@ static void __init new_kmalloc_cache(int idx, slab_flags_t flags)
*/
void __init create_kmalloc_caches(slab_flags_t flags)
{
- int i;
- int type = KMALLOC_NORMAL;
+ int i, type;
- for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
- if (!kmalloc_caches[type][i])
- new_kmalloc_cache(i, flags);
+ for (type = KMALLOC_NORMAL; type <= KMALLOC_RECLAIM; type++) {
+ for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
+ if (!kmalloc_caches[type][i])
+ new_kmalloc_cache(i, type, flags);
- /*
- * Caches that are not of the two-to-the-power-of size.
- * These have to be created immediately after the
- * earlier power of two caches
- */
- if (KMALLOC_MIN_SIZE <= 32 && !kmalloc_caches[type][1] && i == 6)
- new_kmalloc_cache(1, flags);
- if (KMALLOC_MIN_SIZE <= 64 && !kmalloc_caches[type][2] && i == 7)
- new_kmalloc_cache(2, flags);
+ /*
+ * Caches that are not of the two-to-the-power-of size.
+ * These have to be created immediately after the
+ * earlier power of two caches
+ */
+ if (KMALLOC_MIN_SIZE <= 32 && i == 6 &&
+ !kmalloc_caches[type][1])
+ new_kmalloc_cache(1, type, flags);
+ if (KMALLOC_MIN_SIZE <= 64 && i == 7 &&
+ !kmalloc_caches[type][2])
+ new_kmalloc_cache(2, type, flags);
+ }
}
/* Kmalloc array is now usable */
--
2.18.0
^ permalink raw reply related
* [PATCH v3 1/7] mm, slab: combine kmalloc_caches and kmalloc_dma_caches
From: Vlastimil Babka @ 2018-07-18 13:36 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, linux-api, Roman Gushchin, Michal Hocko,
Johannes Weiner, Christoph Lameter, David Rientjes, Joonsoo Kim,
Mel Gorman, Matthew Wilcox, Vlastimil Babka
In-Reply-To: <20180718133620.6205-1-vbabka@suse.cz>
The kmalloc caches currently mainain separate (optional) array
kmalloc_dma_caches for __GFP_DMA allocations. There are tests for __GFP_DMA in
the allocation hotpaths. We can avoid the branches by combining kmalloc_caches
and kmalloc_dma_caches into a single two-dimensional array where the outer
dimension is cache "type". This will also allow to add kmalloc-reclaimable
caches as a third type.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
include/linux/slab.h | 41 ++++++++++++++++++++++++++++++-----------
mm/slab.c | 4 ++--
mm/slab_common.c | 30 +++++++++++-------------------
mm/slub.c | 13 +++++++------
4 files changed, 50 insertions(+), 38 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 14e3fe4bd6a1..4299c59353a1 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -295,12 +295,28 @@ static inline void __check_heap_object(const void *ptr, unsigned long n,
#define SLAB_OBJ_MIN_SIZE (KMALLOC_MIN_SIZE < 16 ? \
(KMALLOC_MIN_SIZE) : 16)
+#define KMALLOC_NORMAL 0
+#ifdef CONFIG_ZONE_DMA
+#define KMALLOC_DMA 1
+#define KMALLOC_TYPES 2
+#else
+#define KMALLOC_TYPES 1
+#endif
+
#ifndef CONFIG_SLOB
-extern struct kmem_cache *kmalloc_caches[KMALLOC_SHIFT_HIGH + 1];
+extern struct kmem_cache *kmalloc_caches[KMALLOC_TYPES][KMALLOC_SHIFT_HIGH + 1];
+
+static __always_inline unsigned int kmalloc_type(gfp_t flags)
+{
+ int is_dma = 0;
+
#ifdef CONFIG_ZONE_DMA
-extern struct kmem_cache *kmalloc_dma_caches[KMALLOC_SHIFT_HIGH + 1];
+ is_dma = !!(flags & __GFP_DMA);
#endif
+ return is_dma;
+}
+
/*
* Figure out which kmalloc slab an allocation of a certain size
* belongs to.
@@ -501,18 +517,20 @@ static __always_inline void *kmalloc_large(size_t size, gfp_t flags)
static __always_inline void *kmalloc(size_t size, gfp_t flags)
{
if (__builtin_constant_p(size)) {
+#ifndef CONFIG_SLOB
+ unsigned int index;
+#endif
if (size > KMALLOC_MAX_CACHE_SIZE)
return kmalloc_large(size, flags);
#ifndef CONFIG_SLOB
- if (!(flags & GFP_DMA)) {
- unsigned int index = kmalloc_index(size);
+ index = kmalloc_index(size);
- if (!index)
- return ZERO_SIZE_PTR;
+ if (!index)
+ return ZERO_SIZE_PTR;
- return kmem_cache_alloc_trace(kmalloc_caches[index],
- flags, size);
- }
+ return kmem_cache_alloc_trace(
+ kmalloc_caches[kmalloc_type(flags)][index],
+ flags, size);
#endif
}
return __kmalloc(size, flags);
@@ -542,13 +560,14 @@ static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
{
#ifndef CONFIG_SLOB
if (__builtin_constant_p(size) &&
- size <= KMALLOC_MAX_CACHE_SIZE && !(flags & GFP_DMA)) {
+ size <= KMALLOC_MAX_CACHE_SIZE) {
unsigned int i = kmalloc_index(size);
if (!i)
return ZERO_SIZE_PTR;
- return kmem_cache_alloc_node_trace(kmalloc_caches[i],
+ return kmem_cache_alloc_node_trace(
+ kmalloc_caches[kmalloc_type(flags)][i],
flags, node, size);
}
#endif
diff --git a/mm/slab.c b/mm/slab.c
index aa76a70e087e..9515798f37b2 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1288,7 +1288,7 @@ void __init kmem_cache_init(void)
* Initialize the caches that provide memory for the kmem_cache_node
* structures first. Without this, further allocations will bug.
*/
- kmalloc_caches[INDEX_NODE] = create_kmalloc_cache(
+ kmalloc_caches[KMALLOC_NORMAL][INDEX_NODE] = create_kmalloc_cache(
kmalloc_info[INDEX_NODE].name,
kmalloc_size(INDEX_NODE), ARCH_KMALLOC_FLAGS,
0, kmalloc_size(INDEX_NODE));
@@ -1304,7 +1304,7 @@ void __init kmem_cache_init(void)
for_each_online_node(nid) {
init_list(kmem_cache, &init_kmem_cache_node[CACHE_CACHE + nid], nid);
- init_list(kmalloc_caches[INDEX_NODE],
+ init_list(kmalloc_caches[KMALLOC_NORMAL][INDEX_NODE],
&init_kmem_cache_node[SIZE_NODE + nid], nid);
}
}
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 2296caf87bfb..4614248ca381 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -973,14 +973,9 @@ struct kmem_cache *__init create_kmalloc_cache(const char *name,
return s;
}
-struct kmem_cache *kmalloc_caches[KMALLOC_SHIFT_HIGH + 1] __ro_after_init;
+struct kmem_cache *kmalloc_caches[KMALLOC_TYPES][KMALLOC_SHIFT_HIGH + 1] __ro_after_init;
EXPORT_SYMBOL(kmalloc_caches);
-#ifdef CONFIG_ZONE_DMA
-struct kmem_cache *kmalloc_dma_caches[KMALLOC_SHIFT_HIGH + 1] __ro_after_init;
-EXPORT_SYMBOL(kmalloc_dma_caches);
-#endif
-
/*
* Conversion table for small slabs sizes / 8 to the index in the
* kmalloc array. This is necessary for slabs < 192 since we have non power
@@ -1040,12 +1035,7 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
} else
index = fls(size - 1);
-#ifdef CONFIG_ZONE_DMA
- if (unlikely((flags & GFP_DMA)))
- return kmalloc_dma_caches[index];
-
-#endif
- return kmalloc_caches[index];
+ return kmalloc_caches[kmalloc_type(flags)][index];
}
/*
@@ -1119,7 +1109,8 @@ void __init setup_kmalloc_cache_index_table(void)
static void __init new_kmalloc_cache(int idx, slab_flags_t flags)
{
- kmalloc_caches[idx] = create_kmalloc_cache(kmalloc_info[idx].name,
+ kmalloc_caches[KMALLOC_NORMAL][idx] = create_kmalloc_cache(
+ kmalloc_info[idx].name,
kmalloc_info[idx].size, flags, 0,
kmalloc_info[idx].size);
}
@@ -1132,9 +1123,10 @@ static void __init new_kmalloc_cache(int idx, slab_flags_t flags)
void __init create_kmalloc_caches(slab_flags_t flags)
{
int i;
+ int type = KMALLOC_NORMAL;
for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
- if (!kmalloc_caches[i])
+ if (!kmalloc_caches[type][i])
new_kmalloc_cache(i, flags);
/*
@@ -1142,9 +1134,9 @@ void __init create_kmalloc_caches(slab_flags_t flags)
* These have to be created immediately after the
* earlier power of two caches
*/
- if (KMALLOC_MIN_SIZE <= 32 && !kmalloc_caches[1] && i == 6)
+ if (KMALLOC_MIN_SIZE <= 32 && !kmalloc_caches[type][1] && i == 6)
new_kmalloc_cache(1, flags);
- if (KMALLOC_MIN_SIZE <= 64 && !kmalloc_caches[2] && i == 7)
+ if (KMALLOC_MIN_SIZE <= 64 && !kmalloc_caches[type][2] && i == 7)
new_kmalloc_cache(2, flags);
}
@@ -1153,7 +1145,7 @@ void __init create_kmalloc_caches(slab_flags_t flags)
#ifdef CONFIG_ZONE_DMA
for (i = 0; i <= KMALLOC_SHIFT_HIGH; i++) {
- struct kmem_cache *s = kmalloc_caches[i];
+ struct kmem_cache *s = kmalloc_caches[KMALLOC_NORMAL][i];
if (s) {
unsigned int size = kmalloc_size(i);
@@ -1161,8 +1153,8 @@ void __init create_kmalloc_caches(slab_flags_t flags)
"dma-kmalloc-%u", size);
BUG_ON(!n);
- kmalloc_dma_caches[i] = create_kmalloc_cache(n,
- size, SLAB_CACHE_DMA | flags, 0, 0);
+ kmalloc_caches[KMALLOC_DMA][i] = create_kmalloc_cache(
+ n, size, SLAB_CACHE_DMA | flags, 0, 0);
}
}
#endif
diff --git a/mm/slub.c b/mm/slub.c
index 51258eff4178..a7b4657ea8e0 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4659,6 +4659,7 @@ static int list_locations(struct kmem_cache *s, char *buf,
static void __init resiliency_test(void)
{
u8 *p;
+ int type = KMALLOC_NORMAL;
BUILD_BUG_ON(KMALLOC_MIN_SIZE > 16 || KMALLOC_SHIFT_HIGH < 10);
@@ -4671,7 +4672,7 @@ static void __init resiliency_test(void)
pr_err("\n1. kmalloc-16: Clobber Redzone/next pointer 0x12->0x%p\n\n",
p + 16);
- validate_slab_cache(kmalloc_caches[4]);
+ validate_slab_cache(kmalloc_caches[type][4]);
/* Hmmm... The next two are dangerous */
p = kzalloc(32, GFP_KERNEL);
@@ -4680,33 +4681,33 @@ static void __init resiliency_test(void)
p);
pr_err("If allocated object is overwritten then not detectable\n\n");
- validate_slab_cache(kmalloc_caches[5]);
+ validate_slab_cache(kmalloc_caches[type][5]);
p = kzalloc(64, GFP_KERNEL);
p += 64 + (get_cycles() & 0xff) * sizeof(void *);
*p = 0x56;
pr_err("\n3. kmalloc-64: corrupting random byte 0x56->0x%p\n",
p);
pr_err("If allocated object is overwritten then not detectable\n\n");
- validate_slab_cache(kmalloc_caches[6]);
+ validate_slab_cache(kmalloc_caches[type][6]);
pr_err("\nB. Corruption after free\n");
p = kzalloc(128, GFP_KERNEL);
kfree(p);
*p = 0x78;
pr_err("1. kmalloc-128: Clobber first word 0x78->0x%p\n\n", p);
- validate_slab_cache(kmalloc_caches[7]);
+ validate_slab_cache(kmalloc_caches[type][7]);
p = kzalloc(256, GFP_KERNEL);
kfree(p);
p[50] = 0x9a;
pr_err("\n2. kmalloc-256: Clobber 50th byte 0x9a->0x%p\n\n", p);
- validate_slab_cache(kmalloc_caches[8]);
+ validate_slab_cache(kmalloc_caches[type][8]);
p = kzalloc(512, GFP_KERNEL);
kfree(p);
p[512] = 0xab;
pr_err("\n3. kmalloc-512: Clobber redzone 0xab->0x%p\n\n", p);
- validate_slab_cache(kmalloc_caches[9]);
+ validate_slab_cache(kmalloc_caches[type][9]);
}
#else
#ifdef CONFIG_SYSFS
--
2.18.0
^ permalink raw reply related
* [PATCH v3 0/7] kmalloc-reclaimable caches
From: Vlastimil Babka @ 2018-07-18 13:36 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, linux-api, Roman Gushchin, Michal Hocko,
Johannes Weiner, Christoph Lameter, David Rientjes, Joonsoo Kim,
Mel Gorman, Matthew Wilcox, Vlastimil Babka, Laura Abbott,
Sumit Semwal, Vijayanand Jitta
v3 changes:
- fix missing hunk in patch 5/7
- more verbose cover letter and patch 6/7 commit log
v2 changes:
- shorten cache names to kmalloc-rcl-<SIZE>
- last patch shortens <SIZE> for all kmalloc caches to e.g. "1k", "4M"
- include dma caches to the 2D kmalloc_caches[] array to avoid a branch
- vmstat counter nr_indirectly_reclaimable_bytes renamed to
nr_kernel_misc_reclaimable, doesn't include kmalloc-rcl-*
- /proc/meminfo counter renamed to KReclaimable, includes kmalloc-rcl*
and nr_kernel_misc_reclaimable
Hi,
as discussed at LSF/MM [1] here's a patchset that introduces
kmalloc-reclaimable caches (more details in the second patch) and uses them for
SLAB freelists and dcache external names. The latter allows us to repurpose the
NR_INDIRECTLY_RECLAIMABLE_BYTES counter later in the series.
With patch 4/7, dcache external names are allocated from kmalloc-rcl-*
caches, eliminating the need for manual accounting. More importantly, it
also ensures the reclaimable kmalloc allocations are grouped in pages
separate from the regular kmalloc allocations. The need for proper
accounting of dcache external names has shown it's easy for misbehaving
process to allocate lots of them, causing premature OOMs. Without the
added grouping, it's likely that a similar workload can interleave the
dcache external names allocations with regular kmalloc allocations
(note: I haven't searched myself for an example of such regular kmalloc
allocation, but I would be very surprised if there wasn't some). A
pathological case would be e.g. one 64byte regular allocations with 63
external dcache names in a page (64x64=4096), which means the page is
not freed even after reclaiming after all dcache names, and the process
can thus "steal" the whole page with single 64byte allocation.
If there other kmalloc users similar to dcache external names become
identified, they can also benefit from the new functionality simply by
adding __GFP_RECLAIMABLE to the kmalloc calls.
Side benefits of the patchset (that could be also merged separately)
include removed branch for detecting __GFP_DMA kmalloc(), and shortening
kmalloc cache names in /proc/slabinfo output. The latter is potentially
an ABI break in case there are tools parsing the names and expecting the
values to be in bytes.
This is how /proc/slabinfo looks like after booting in virtme:
...
kmalloc-rcl-4M 0 0 4194304 1 1024 : tunables 1 1 0 : slabdata 0 0 0
...
kmalloc-rcl-96 7 32 128 32 1 : tunables 120 60 8 : slabdata 1 1 0
kmalloc-rcl-64 25 128 64 64 1 : tunables 120 60 8 : slabdata 2 2 0
kmalloc-rcl-32 0 0 32 124 1 : tunables 120 60 8 : slabdata 0 0 0
kmalloc-4M 0 0 4194304 1 1024 : tunables 1 1 0 : slabdata 0 0 0
kmalloc-2M 0 0 2097152 1 512 : tunables 1 1 0 : slabdata 0 0 0
kmalloc-1M 0 0 1048576 1 256 : tunables 1 1 0 : slabdata 0 0 0
...
/proc/vmstat with renamed nr_indirectly_reclaimable_bytes counter:
...
nr_slab_reclaimable 2817
nr_slab_unreclaimable 1781
...
nr_kernel_misc_reclaimable 0
...
/proc/meminfo with new KReclaimable counter:
...
Shmem: 564 kB
KReclaimable: 11260 kB
Slab: 18368 kB
SReclaimable: 11260 kB
SUnreclaim: 7108 kB
KernelStack: 1248 kB
...
Thanks,
Vlastimil
Vlastimil Babka (7):
mm, slab: combine kmalloc_caches and kmalloc_dma_caches
mm, slab/slub: introduce kmalloc-reclaimable caches
mm, slab: allocate off-slab freelists as reclaimable when appropriate
dcache: allocate external names from reclaimable kmalloc caches
mm: rename and change semantics of nr_indirectly_reclaimable_bytes
mm, proc: add KReclaimable to /proc/meminfo
mm, slab: shorten kmalloc cache names for large sizes
Documentation/filesystems/proc.txt | 4 +
drivers/base/node.c | 19 ++--
drivers/staging/android/ion/ion_page_pool.c | 8 +-
fs/dcache.c | 38 ++------
fs/proc/meminfo.c | 16 +--
include/linux/mmzone.h | 2 +-
include/linux/slab.h | 49 +++++++---
mm/page_alloc.c | 19 ++--
mm/slab.c | 11 ++-
mm/slab_common.c | 102 ++++++++++++--------
mm/slub.c | 13 +--
mm/util.c | 3 +-
mm/vmstat.c | 6 +-
13 files changed, 161 insertions(+), 129 deletions(-)
--
2.18.0
^ permalink raw reply
* [PATCH 6/6] blkcg: Track DISCARD statistics and output them in cgroup io.stat
From: Tejun Heo @ 2018-07-18 11:47 UTC (permalink / raw)
To: axboe
Cc: michaelcallahan, newella, linux-block, linux-kernel, kernel-team,
linux-api, Tejun Heo
In-Reply-To: <20180718114741.2580313-1-tj@kernel.org>
Add tracking of REQ_OP_DISCARD ios to the per-cgroup io.stat. Two
fields, dbytes and dios, to respectively count the total bytes and
number of discards are added.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Andy Newell <newella@fb.com>
Cc: Michael Callahan <michaelcallahan@fb.com>
---
Documentation/admin-guide/cgroup-v2.rst | 10 ++++++----
block/blk-cgroup.c | 13 +++++++++----
include/linux/blk-cgroup.h | 5 ++++-
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 569ce27b85e5..3afe10fa82bc 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -1317,17 +1317,19 @@ IO Interface Files
Lines are keyed by $MAJ:$MIN device numbers and not ordered.
The following nested keys are defined.
- ====== ===================
+ ====== =====================
rbytes Bytes read
wbytes Bytes written
rios Number of read IOs
wios Number of write IOs
- ====== ===================
+ dbytes Bytes discarded
+ dios Number of discard IOs
+ ====== =====================
An example read output follows:
- 8:16 rbytes=1459200 wbytes=314773504 rios=192 wios=353
- 8:0 rbytes=90430464 wbytes=299008000 rios=8950 wios=1252
+ 8:16 rbytes=1459200 wbytes=314773504 rios=192 wios=353 dbytes=0 dios=0
+ 8:0 rbytes=90430464 wbytes=299008000 rios=8950 wios=1252 dbytes=50331648 dios=3021
io.weight
A read-write flat-keyed file which exists on non-root cgroups.
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 7e2c19ce1a08..1942357d7165 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -567,6 +567,7 @@ u64 __blkg_prfill_rwstat(struct seq_file *sf, struct blkg_policy_data *pd,
[BLKG_RWSTAT_WRITE] = "Write",
[BLKG_RWSTAT_SYNC] = "Sync",
[BLKG_RWSTAT_ASYNC] = "Async",
+ [BLKG_RWSTAT_DISCARD] = "Discard",
};
const char *dname = blkg_dev_name(pd->blkg);
u64 v;
@@ -580,7 +581,8 @@ u64 __blkg_prfill_rwstat(struct seq_file *sf, struct blkg_policy_data *pd,
(unsigned long long)atomic64_read(&rwstat->aux_cnt[i]));
v = atomic64_read(&rwstat->aux_cnt[BLKG_RWSTAT_READ]) +
- atomic64_read(&rwstat->aux_cnt[BLKG_RWSTAT_WRITE]);
+ atomic64_read(&rwstat->aux_cnt[BLKG_RWSTAT_WRITE]) +
+ atomic64_read(&rwstat->aux_cnt[BLKG_RWSTAT_DISCARD]);
seq_printf(sf, "%s Total %llu\n", dname, (unsigned long long)v);
return v;
}
@@ -959,7 +961,7 @@ static int blkcg_print_stat(struct seq_file *sf, void *v)
const char *dname;
char *buf;
struct blkg_rwstat rwstat;
- u64 rbytes, wbytes, rios, wios;
+ u64 rbytes, wbytes, rios, wios, dbytes, dios;
size_t size = seq_get_buf(sf, &buf), off = 0;
int i;
bool has_stats = false;
@@ -982,19 +984,22 @@ static int blkcg_print_stat(struct seq_file *sf, void *v)
offsetof(struct blkcg_gq, stat_bytes));
rbytes = atomic64_read(&rwstat.aux_cnt[BLKG_RWSTAT_READ]);
wbytes = atomic64_read(&rwstat.aux_cnt[BLKG_RWSTAT_WRITE]);
+ dbytes = atomic64_read(&rwstat.aux_cnt[BLKG_RWSTAT_DISCARD]);
rwstat = blkg_rwstat_recursive_sum(blkg, NULL,
offsetof(struct blkcg_gq, stat_ios));
rios = atomic64_read(&rwstat.aux_cnt[BLKG_RWSTAT_READ]);
wios = atomic64_read(&rwstat.aux_cnt[BLKG_RWSTAT_WRITE]);
+ dios = atomic64_read(&rwstat.aux_cnt[BLKG_RWSTAT_DISCARD]);
spin_unlock_irq(blkg->q->queue_lock);
if (rbytes || wbytes || rios || wios) {
has_stats = true;
off += scnprintf(buf+off, size-off,
- "rbytes=%llu wbytes=%llu rios=%llu wios=%llu",
- rbytes, wbytes, rios, wios);
+ "rbytes=%llu wbytes=%llu rios=%llu wios=%llu dbytes=%llu dios=%llu",
+ rbytes, wbytes, rios, wios,
+ dbytes, dios);
}
if (!blkcg_debug_stats)
diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h
index de57de4831d5..3bed5e02a873 100644
--- a/include/linux/blk-cgroup.h
+++ b/include/linux/blk-cgroup.h
@@ -35,6 +35,7 @@ enum blkg_rwstat_type {
BLKG_RWSTAT_WRITE,
BLKG_RWSTAT_SYNC,
BLKG_RWSTAT_ASYNC,
+ BLKG_RWSTAT_DISCARD,
BLKG_RWSTAT_NR,
BLKG_RWSTAT_TOTAL = BLKG_RWSTAT_NR,
@@ -649,7 +650,9 @@ static inline void blkg_rwstat_add(struct blkg_rwstat *rwstat,
{
struct percpu_counter *cnt;
- if (op_is_write(op))
+ if (op_is_discard(op))
+ cnt = &rwstat->cpu_cnt[BLKG_RWSTAT_DISCARD];
+ else if (op_is_write(op))
cnt = &rwstat->cpu_cnt[BLKG_RWSTAT_WRITE];
else
cnt = &rwstat->cpu_cnt[BLKG_RWSTAT_READ];
--
2.17.1
^ permalink raw reply related
* [PATCH 5/6] block: Track DISCARD statistics and output them in stat and diskstat
From: Tejun Heo @ 2018-07-18 11:47 UTC (permalink / raw)
To: axboe
Cc: michaelcallahan, newella, linux-block, linux-kernel, kernel-team,
linux-api, Tejun Heo
In-Reply-To: <20180718114741.2580313-1-tj@kernel.org>
From: Michael Callahan <michaelcallahan@fb.com>
Add tracking of REQ_OP_DISCARD ios to the partition statistics and
append them to the various stat files in /sys as well as
/proc/diskstats. These are tracked with the same four stats as reads
and writes:
Number of discard ios completed.
Number of discard ios merged
Number of discard sectors completed
Milliseconds spent on discard requests
This is done via adding a new STAT_DISCARD define to genhd.h and then
using it to index that stat field for discard requests.
tj: Refreshed on top of v4.17 and other previous updates.
Signed-off-by: Michael Callahan <michaelcallahan@fb.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Andy Newell <newella@fb.com>
---
Documentation/ABI/testing/procfs-diskstats | 10 ++++++++
Documentation/block/stat.txt | 28 ++++++++++++----------
Documentation/iostats.txt | 15 ++++++++++++
block/genhd.c | 13 +++++++---
block/partition-generic.c | 9 +++++--
include/linux/blk_types.h | 8 +++++++
include/linux/genhd.h | 3 ++-
7 files changed, 68 insertions(+), 18 deletions(-)
diff --git a/Documentation/ABI/testing/procfs-diskstats b/Documentation/ABI/testing/procfs-diskstats
index f91a973a37fe..abac31d216de 100644
--- a/Documentation/ABI/testing/procfs-diskstats
+++ b/Documentation/ABI/testing/procfs-diskstats
@@ -5,6 +5,7 @@ Contact: Jerome Marchand <jmarchan@redhat.com>
The /proc/diskstats file displays the I/O statistics
of block devices. Each line contains the following 14
fields:
+
1 - major number
2 - minor mumber
3 - device name
@@ -19,4 +20,13 @@ Contact: Jerome Marchand <jmarchan@redhat.com>
12 - I/Os currently in progress
13 - time spent doing I/Os (ms)
14 - weighted time spent doing I/Os (ms)
+
+ Kernel 4.18+ appends four more fields for discard
+ tracking putting the total at 18:
+
+ 15 - discards completed successfully
+ 16 - discards merged
+ 17 - sectors discarded
+ 18 - time spent discarding
+
For more details refer to Documentation/iostats.txt
diff --git a/Documentation/block/stat.txt b/Documentation/block/stat.txt
index 0dbc946de2ea..0aace9cc536c 100644
--- a/Documentation/block/stat.txt
+++ b/Documentation/block/stat.txt
@@ -31,28 +31,32 @@ write ticks milliseconds total wait time for write requests
in_flight requests number of I/Os currently in flight
io_ticks milliseconds total time this block device has been active
time_in_queue milliseconds total wait time for all requests
+discard I/Os requests number of discard I/Os processed
+discard merges requests number of discard I/Os merged with in-queue I/O
+discard sectors sectors number of sectors discarded
+discard ticks milliseconds total wait time for discard requests
-read I/Os, write I/Os
-=====================
+read I/Os, write I/Os, discard I/0s
+===================================
These values increment when an I/O request completes.
-read merges, write merges
-=========================
+read merges, write merges, discard merges
+=========================================
These values increment when an I/O request is merged with an
already-queued I/O request.
-read sectors, write sectors
-===========================
+read sectors, write sectors, discard_sectors
+============================================
-These values count the number of sectors read from or written to this
-block device. The "sectors" in question are the standard UNIX 512-byte
-sectors, not any device- or filesystem-specific block size. The
-counters are incremented when the I/O completes.
+These values count the number of sectors read from, written to, or
+discarded from this block device. The "sectors" in question are the
+standard UNIX 512-byte sectors, not any device- or filesystem-specific
+block size. The counters are incremented when the I/O completes.
-read ticks, write ticks
-=======================
+read ticks, write ticks, discard ticks
+======================================
These values count the number of milliseconds that I/O requests have
waited on this block device. If there are multiple I/O requests waiting,
diff --git a/Documentation/iostats.txt b/Documentation/iostats.txt
index 04d394a2e06c..49df45f90e8a 100644
--- a/Documentation/iostats.txt
+++ b/Documentation/iostats.txt
@@ -31,6 +31,9 @@ and so should not differ.
3 0 hda 446216 784926 9550688 4382310 424847 312726 5922052 19310380 0 3376340 23705160
3 1 hda1 35486 38030 38030 38030
+ 4.18+ diskstats:
+ 3 0 hda 446216 784926 9550688 4382310 424847 312726 5922052 19310380 0 3376340 23705160 0 0 0 0
+
On 2.4 you might execute ``grep 'hda ' /proc/partitions``. On 2.6+, you have
a choice of ``cat /sys/block/hda/stat`` or ``grep 'hda ' /proc/diskstats``.
@@ -101,6 +104,18 @@ Field 11 -- weighted # of milliseconds spent doing I/Os
last update of this field. This can provide an easy measure of both
I/O completion time and the backlog that may be accumulating.
+Field 12 -- # of discards completed
+ This is the total number of discards completed successfully.
+
+Field 13 -- # of discards merged
+ See the description of field 2
+
+Field 14 -- # of sectors discarded
+ This is the total number of sectors discarded successfully.
+
+Field 15 -- # of milliseconds spent discarding
+ This is the total number of milliseconds spent by all discards (as
+ measured from __make_request() to end_that_request_last()).
To avoid introducing performance bottlenecks, no locks are held while
modifying these counters. This implies that minor inaccuracies may be
diff --git a/block/genhd.c b/block/genhd.c
index 0711a800d0d4..8cc719a37b32 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1333,8 +1333,11 @@ static int diskstats_show(struct seq_file *seqf, void *v)
part_round_stats(gp->queue, cpu, hd);
part_stat_unlock();
part_in_flight(gp->queue, hd, inflight);
- seq_printf(seqf, "%4d %7d %s %lu %lu %lu "
- "%u %lu %lu %lu %u %u %u %u\n",
+ seq_printf(seqf, "%4d %7d %s "
+ "%lu %lu %lu %u "
+ "%lu %lu %lu %u "
+ "%u %u %u "
+ "%lu %lu %lu %u\n",
MAJOR(part_devt(hd)), MINOR(part_devt(hd)),
disk_name(gp, hd->partno, buf),
part_stat_read(hd, ios[STAT_READ]),
@@ -1347,7 +1350,11 @@ static int diskstats_show(struct seq_file *seqf, void *v)
jiffies_to_msecs(part_stat_read(hd, ticks[STAT_WRITE])),
inflight[0],
jiffies_to_msecs(part_stat_read(hd, io_ticks)),
- jiffies_to_msecs(part_stat_read(hd, time_in_queue))
+ jiffies_to_msecs(part_stat_read(hd, time_in_queue)),
+ part_stat_read(hd, ios[STAT_DISCARD]),
+ part_stat_read(hd, merges[STAT_DISCARD]),
+ part_stat_read(hd, sectors[STAT_DISCARD]),
+ jiffies_to_msecs(part_stat_read(hd, ticks[STAT_DISCARD]))
);
}
disk_part_iter_exit(&piter);
diff --git a/block/partition-generic.c b/block/partition-generic.c
index 0ddb06722162..5a8975a1201c 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -130,7 +130,8 @@ ssize_t part_stat_show(struct device *dev,
return sprintf(buf,
"%8lu %8lu %8llu %8u "
"%8lu %8lu %8llu %8u "
- "%8u %8u %8u"
+ "%8u %8u %8u "
+ "%8lu %8lu %8llu %8u"
"\n",
part_stat_read(p, ios[STAT_READ]),
part_stat_read(p, merges[STAT_READ]),
@@ -142,7 +143,11 @@ ssize_t part_stat_show(struct device *dev,
jiffies_to_msecs(part_stat_read(p, ticks[STAT_WRITE])),
inflight[0],
jiffies_to_msecs(part_stat_read(p, io_ticks)),
- jiffies_to_msecs(part_stat_read(p, time_in_queue)));
+ jiffies_to_msecs(part_stat_read(p, time_in_queue)),
+ part_stat_read(p, ios[STAT_DISCARD]),
+ part_stat_read(p, merges[STAT_DISCARD]),
+ (unsigned long long)part_stat_read(p, sectors[STAT_DISCARD]),
+ jiffies_to_msecs(part_stat_read(p, ticks[STAT_DISCARD])));
}
ssize_t part_inflight_show(struct device *dev, struct device_attribute *attr,
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 2960a96d833c..f6dfb30737d8 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -360,6 +360,7 @@ enum req_flag_bits {
enum stat_group {
STAT_READ,
STAT_WRITE,
+ STAT_DISCARD,
NR_STAT_GROUPS
};
@@ -401,8 +402,15 @@ static inline bool op_is_sync(unsigned int op)
(op & (REQ_SYNC | REQ_FUA | REQ_PREFLUSH));
}
+static inline bool op_is_discard(unsigned int op)
+{
+ return (op & REQ_OP_MASK) == REQ_OP_DISCARD;
+}
+
static inline int op_stat_group(unsigned int op)
{
+ if (op_is_discard(op))
+ return STAT_DISCARD;
return op_is_write(op);
}
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index a75445446974..57864422a2c8 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -356,7 +356,8 @@ static inline void free_part_stats(struct hd_struct *part)
#define part_stat_read_accum(part, field) \
(part_stat_read(part, field[STAT_READ]) + \
- part_stat_read(part, field[STAT_WRITE]))
+ part_stat_read(part, field[STAT_WRITE]) + \
+ part_stat_read(part, field[STAT_DISCARD]))
#define part_stat_add(cpu, part, field, addnd) do { \
__part_stat_add((cpu), (part), field, addnd); \
--
2.17.1
^ permalink raw reply related
* [PATCH 4/6] block: Add and use op_stat_group() for indexing disk_stat fields.
From: Tejun Heo @ 2018-07-18 11:47 UTC (permalink / raw)
To: axboe
Cc: michaelcallahan, newella, linux-block, linux-kernel, kernel-team,
linux-api, Tejun Heo, Minchan Kim, Dan Williams, Joshua Morris,
Philipp Reisner, Matias Bjorling, Kent Overstreet,
Alasdair Kergon
In-Reply-To: <20180718114741.2580313-1-tj@kernel.org>
From: Michael Callahan <michaelcallahan@fb.com>
Add and use a new op_stat_group() function for indexing partition stat
fields rather than indexing them by rq_data_dir() or bio_data_dir().
This function works similarly to op_is_sync() in that it takes the
request::cmd_flags or bio::bi_opf flags and determines which stats
should et updated.
In addition, the second parameter to generic_start_io_acct() and
generic_end_io_acct() is now a REQ_OP rather than simply a read or
write bit and it uses op_stat_group() on the parameter to determine
the stat group.
Note that the partition in_flight counts are not part of the per-cpu
statistics and as such are not indexed via this function. It's now
indexed by op_is_write().
tj: Refreshed on top of v4.17. Updated to pass around REQ_OP.
Signed-off-by: Michael Callahan <michaelcallahan@fb.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Joshua Morris <josh.h.morris@us.ibm.com>
Cc: Philipp Reisner <philipp.reisner@linbit.com>
Cc: Matias Bjorling <mb@lightnvm.io>
Cc: Kent Overstreet <kent.overstreet@gmail.com>
Cc: Alasdair Kergon <agk@redhat.com>
---
block/bio.c | 16 +++++++++-------
block/blk-core.c | 12 ++++++------
drivers/block/drbd/drbd_req.c | 4 ++--
drivers/block/rsxx/dev.c | 6 +++---
drivers/block/zram/zram_drv.c | 5 ++---
drivers/lightnvm/pblk-cache.c | 5 +++--
drivers/lightnvm/pblk-read.c | 5 +++--
drivers/md/bcache/request.c | 13 +++++--------
drivers/md/dm.c | 6 ++++--
drivers/md/md.c | 5 +++--
drivers/nvdimm/nd.h | 7 +++----
include/linux/bio.h | 4 ++--
include/linux/blk_types.h | 5 +++++
13 files changed, 50 insertions(+), 43 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index f3536bfc8298..8ecc95615941 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1728,29 +1728,31 @@ void bio_check_pages_dirty(struct bio *bio)
}
EXPORT_SYMBOL_GPL(bio_check_pages_dirty);
-void generic_start_io_acct(struct request_queue *q, int rw,
+void generic_start_io_acct(struct request_queue *q, int op,
unsigned long sectors, struct hd_struct *part)
{
+ const int sgrp = op_stat_group(op);
int cpu = part_stat_lock();
part_round_stats(q, cpu, part);
- part_stat_inc(cpu, part, ios[rw]);
- part_stat_add(cpu, part, sectors[rw], sectors);
- part_inc_in_flight(q, part, rw);
+ part_stat_inc(cpu, part, ios[sgrp]);
+ part_stat_add(cpu, part, sectors[sgrp], sectors);
+ part_inc_in_flight(q, part, op_is_write(op));
part_stat_unlock();
}
EXPORT_SYMBOL(generic_start_io_acct);
-void generic_end_io_acct(struct request_queue *q, int rw,
+void generic_end_io_acct(struct request_queue *q, int req_op,
struct hd_struct *part, unsigned long start_time)
{
unsigned long duration = jiffies - start_time;
+ const int sgrp = op_stat_group(req_op);
int cpu = part_stat_lock();
- part_stat_add(cpu, part, ticks[rw], duration);
+ part_stat_add(cpu, part, ticks[sgrp], duration);
part_round_stats(q, cpu, part);
- part_dec_in_flight(q, part, rw);
+ part_dec_in_flight(q, part, op_is_write(req_op));
part_stat_unlock();
}
diff --git a/block/blk-core.c b/block/blk-core.c
index c4b57d8806fe..03a4ea93a5f3 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2702,13 +2702,13 @@ EXPORT_SYMBOL_GPL(blk_rq_err_bytes);
void blk_account_io_completion(struct request *req, unsigned int bytes)
{
if (blk_do_io_stat(req)) {
- const int rw = rq_data_dir(req);
+ const int sgrp = op_stat_group(req_op(req));
struct hd_struct *part;
int cpu;
cpu = part_stat_lock();
part = req->part;
- part_stat_add(cpu, part, sectors[rw], bytes >> 9);
+ part_stat_add(cpu, part, sectors[sgrp], bytes >> 9);
part_stat_unlock();
}
}
@@ -2722,7 +2722,7 @@ void blk_account_io_done(struct request *req, u64 now)
*/
if (blk_do_io_stat(req) && !(req->rq_flags & RQF_FLUSH_SEQ)) {
unsigned long duration;
- const int rw = rq_data_dir(req);
+ const int sgrp = op_stat_group(req_op(req));
struct hd_struct *part;
int cpu;
@@ -2730,10 +2730,10 @@ void blk_account_io_done(struct request *req, u64 now)
cpu = part_stat_lock();
part = req->part;
- part_stat_inc(cpu, part, ios[rw]);
- part_stat_add(cpu, part, ticks[rw], duration);
+ part_stat_inc(cpu, part, ios[sgrp]);
+ part_stat_add(cpu, part, ticks[sgrp], duration);
part_round_stats(req->q, cpu, part);
- part_dec_in_flight(req->q, part, rw);
+ part_dec_in_flight(req->q, part, rq_data_dir(req));
hd_struct_put(part);
part_stat_unlock();
diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
index d146fedc38bb..19cac36e9737 100644
--- a/drivers/block/drbd/drbd_req.c
+++ b/drivers/block/drbd/drbd_req.c
@@ -38,7 +38,7 @@ static void _drbd_start_io_acct(struct drbd_device *device, struct drbd_request
{
struct request_queue *q = device->rq_queue;
- generic_start_io_acct(q, bio_data_dir(req->master_bio),
+ generic_start_io_acct(q, bio_op(req->master_bio),
req->i.size >> 9, &device->vdisk->part0);
}
@@ -47,7 +47,7 @@ static void _drbd_end_io_acct(struct drbd_device *device, struct drbd_request *r
{
struct request_queue *q = device->rq_queue;
- generic_end_io_acct(q, bio_data_dir(req->master_bio),
+ generic_end_io_acct(q, bio_op(req->master_bio),
&device->vdisk->part0, req->start_jif);
}
diff --git a/drivers/block/rsxx/dev.c b/drivers/block/rsxx/dev.c
index dddb3f2490b6..1a92f9e65937 100644
--- a/drivers/block/rsxx/dev.c
+++ b/drivers/block/rsxx/dev.c
@@ -112,7 +112,7 @@ static const struct block_device_operations rsxx_fops = {
static void disk_stats_start(struct rsxx_cardinfo *card, struct bio *bio)
{
- generic_start_io_acct(card->queue, bio_data_dir(bio), bio_sectors(bio),
+ generic_start_io_acct(card->queue, bio_op(bio), bio_sectors(bio),
&card->gendisk->part0);
}
@@ -120,8 +120,8 @@ static void disk_stats_complete(struct rsxx_cardinfo *card,
struct bio *bio,
unsigned long start_time)
{
- generic_end_io_acct(card->queue, bio_data_dir(bio),
- &card->gendisk->part0, start_time);
+ generic_end_io_acct(card->queue, bio_op(bio),
+ &card->gendisk->part0, start_time);
}
static void bio_dma_done_cb(struct rsxx_cardinfo *card,
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 78c29044684a..2907a8156aaf 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1277,11 +1277,10 @@ static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
int offset, unsigned int op, struct bio *bio)
{
unsigned long start_time = jiffies;
- int rw_acct = op_is_write(op) ? REQ_OP_WRITE : REQ_OP_READ;
struct request_queue *q = zram->disk->queue;
int ret;
- generic_start_io_acct(q, rw_acct, bvec->bv_len >> SECTOR_SHIFT,
+ generic_start_io_acct(q, op, bvec->bv_len >> SECTOR_SHIFT,
&zram->disk->part0);
if (!op_is_write(op)) {
@@ -1293,7 +1292,7 @@ static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
ret = zram_bvec_write(zram, bvec, index, offset, bio);
}
- generic_end_io_acct(q, rw_acct, &zram->disk->part0, start_time);
+ generic_end_io_acct(q, op, &zram->disk->part0, start_time);
zram_slot_lock(zram, index);
zram_accessed(zram, index);
diff --git a/drivers/lightnvm/pblk-cache.c b/drivers/lightnvm/pblk-cache.c
index 77d811962818..f565a56b898a 100644
--- a/drivers/lightnvm/pblk-cache.c
+++ b/drivers/lightnvm/pblk-cache.c
@@ -27,7 +27,8 @@ int pblk_write_to_cache(struct pblk *pblk, struct bio *bio, unsigned long flags)
int nr_entries = pblk_get_secs(bio);
int i, ret;
- generic_start_io_acct(q, WRITE, bio_sectors(bio), &pblk->disk->part0);
+ generic_start_io_acct(q, REQ_OP_WRITE, bio_sectors(bio),
+ &pblk->disk->part0);
/* Update the write buffer head (mem) with the entries that we can
* write. The write in itself cannot fail, so there is no need to
@@ -75,7 +76,7 @@ int pblk_write_to_cache(struct pblk *pblk, struct bio *bio, unsigned long flags)
pblk_rl_inserted(&pblk->rl, nr_entries);
out:
- generic_end_io_acct(q, WRITE, &pblk->disk->part0, start_time);
+ generic_end_io_acct(q, REQ_OP_WRITE, &pblk->disk->part0, start_time);
pblk_write_should_kick(pblk);
return ret;
}
diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
index 26d414ae25b6..5a46d7f9302f 100644
--- a/drivers/lightnvm/pblk-read.c
+++ b/drivers/lightnvm/pblk-read.c
@@ -199,7 +199,7 @@ static void __pblk_end_io_read(struct pblk *pblk, struct nvm_rq *rqd,
struct bio *int_bio = rqd->bio;
unsigned long start_time = r_ctx->start_time;
- generic_end_io_acct(dev->q, READ, &pblk->disk->part0, start_time);
+ generic_end_io_acct(dev->q, REQ_OP_READ, &pblk->disk->part0, start_time);
if (rqd->error)
pblk_log_read_err(pblk, rqd);
@@ -461,7 +461,8 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio)
return NVM_IO_ERR;
}
- generic_start_io_acct(q, READ, bio_sectors(bio), &pblk->disk->part0);
+ generic_start_io_acct(q, REQ_OP_READ, bio_sectors(bio),
+ &pblk->disk->part0);
bitmap_zero(read_bitmap, nr_secs);
diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index ae67f5fa8047..97707b0c54ce 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -667,8 +667,7 @@ static void backing_request_endio(struct bio *bio)
static void bio_complete(struct search *s)
{
if (s->orig_bio) {
- generic_end_io_acct(s->d->disk->queue,
- bio_data_dir(s->orig_bio),
+ generic_end_io_acct(s->d->disk->queue, bio_op(s->orig_bio),
&s->d->disk->part0, s->start_time);
trace_bcache_request_end(s->d, s->orig_bio);
@@ -1062,8 +1061,7 @@ static void detached_dev_end_io(struct bio *bio)
bio->bi_end_io = ddip->bi_end_io;
bio->bi_private = ddip->bi_private;
- generic_end_io_acct(ddip->d->disk->queue,
- bio_data_dir(bio),
+ generic_end_io_acct(ddip->d->disk->queue, bio_op(bio),
&ddip->d->disk->part0, ddip->start_time);
if (bio->bi_status) {
@@ -1120,7 +1118,7 @@ static blk_qc_t cached_dev_make_request(struct request_queue *q,
}
atomic_set(&dc->backing_idle, 0);
- generic_start_io_acct(q, rw, bio_sectors(bio), &d->disk->part0);
+ generic_start_io_acct(q, bio_op(bio), bio_sectors(bio), &d->disk->part0);
bio_set_dev(bio, dc->bdev);
bio->bi_iter.bi_sector += dc->sb.data_offset;
@@ -1229,7 +1227,6 @@ static blk_qc_t flash_dev_make_request(struct request_queue *q,
struct search *s;
struct closure *cl;
struct bcache_device *d = bio->bi_disk->private_data;
- int rw = bio_data_dir(bio);
if (unlikely(d->c && test_bit(CACHE_SET_IO_DISABLE, &d->c->flags))) {
bio->bi_status = BLK_STS_IOERR;
@@ -1237,7 +1234,7 @@ static blk_qc_t flash_dev_make_request(struct request_queue *q,
return BLK_QC_T_NONE;
}
- generic_start_io_acct(q, rw, bio_sectors(bio), &d->disk->part0);
+ generic_start_io_acct(q, bio_op(bio), bio_sectors(bio), &d->disk->part0);
s = search_alloc(bio, d);
cl = &s->cl;
@@ -1254,7 +1251,7 @@ static blk_qc_t flash_dev_make_request(struct request_queue *q,
flash_dev_nodata,
bcache_wq);
return BLK_QC_T_NONE;
- } else if (rw) {
+ } else if (bio_data_dir(bio)) {
bch_keybuf_check_overlapping(&s->iop.c->moving_gc_keys,
&KEY(d->id, bio->bi_iter.bi_sector, 0),
&KEY(d->id, bio_end_sector(bio), 0));
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index b0dd7027848b..20f7e4ef5342 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -609,7 +609,8 @@ static void start_io_acct(struct dm_io *io)
io->start_time = jiffies;
- generic_start_io_acct(md->queue, rw, bio_sectors(bio), &dm_disk(md)->part0);
+ generic_start_io_acct(md->queue, bio_op(bio), bio_sectors(bio),
+ &dm_disk(md)->part0);
atomic_set(&dm_disk(md)->part0.in_flight[rw],
atomic_inc_return(&md->pending[rw]));
@@ -628,7 +629,8 @@ static void end_io_acct(struct dm_io *io)
int pending;
int rw = bio_data_dir(bio);
- generic_end_io_acct(md->queue, rw, &dm_disk(md)->part0, io->start_time);
+ generic_end_io_acct(md->queue, bio_op(bio), &dm_disk(md)->part0,
+ io->start_time);
if (unlikely(dm_stats_used(&md->stats)))
dm_stats_account_io(&md->stats, bio_data_dir(bio),
diff --git a/drivers/md/md.c b/drivers/md/md.c
index dabe36723d60..f6e58dbca0d4 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -335,6 +335,7 @@ EXPORT_SYMBOL(md_handle_request);
static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
{
const int rw = bio_data_dir(bio);
+ const int sgrp = op_stat_group(bio_op(bio));
struct mddev *mddev = q->queuedata;
unsigned int sectors;
int cpu;
@@ -363,8 +364,8 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
md_handle_request(mddev, bio);
cpu = part_stat_lock();
- part_stat_inc(cpu, &mddev->gendisk->part0, ios[rw]);
- part_stat_add(cpu, &mddev->gendisk->part0, sectors[rw], sectors);
+ part_stat_inc(cpu, &mddev->gendisk->part0, ios[sgrp]);
+ part_stat_add(cpu, &mddev->gendisk->part0, sectors[sgrp], sectors);
part_stat_unlock();
return BLK_QC_T_NONE;
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index 32e0364b48b9..6ee7fd7e4bbd 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -396,16 +396,15 @@ static inline bool nd_iostat_start(struct bio *bio, unsigned long *start)
return false;
*start = jiffies;
- generic_start_io_acct(disk->queue, bio_data_dir(bio),
- bio_sectors(bio), &disk->part0);
+ generic_start_io_acct(disk->queue, bio_op(bio), bio_sectors(bio),
+ &disk->part0);
return true;
}
static inline void nd_iostat_end(struct bio *bio, unsigned long start)
{
struct gendisk *disk = bio->bi_disk;
- generic_end_io_acct(disk->queue, bio_data_dir(bio), &disk->part0,
- start);
+ generic_end_io_acct(disk->queue, bio_op(bio), &disk->part0, start);
}
static inline bool is_bad_pmem(struct badblocks *bb, sector_t sector,
unsigned int len)
diff --git a/include/linux/bio.h b/include/linux/bio.h
index a00dfff51aa5..ab221c517f4e 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -496,9 +496,9 @@ extern struct bio *bio_copy_kern(struct request_queue *, void *, unsigned int,
extern void bio_set_pages_dirty(struct bio *bio);
extern void bio_check_pages_dirty(struct bio *bio);
-void generic_start_io_acct(struct request_queue *q, int rw,
+void generic_start_io_acct(struct request_queue *q, int op,
unsigned long sectors, struct hd_struct *part);
-void generic_end_io_acct(struct request_queue *q, int rw,
+void generic_end_io_acct(struct request_queue *q, int op,
struct hd_struct *part,
unsigned long start_time);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index d2b44de56bc1..2960a96d833c 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -401,6 +401,11 @@ static inline bool op_is_sync(unsigned int op)
(op & (REQ_SYNC | REQ_FUA | REQ_PREFLUSH));
}
+static inline int op_stat_group(unsigned int op)
+{
+ return op_is_write(op);
+}
+
typedef unsigned int blk_qc_t;
#define BLK_QC_T_NONE -1U
#define BLK_QC_T_SHIFT 16
--
2.17.1
^ permalink raw reply related
* [PATCH 3/6] block: Define and use STAT_READ and STAT_WRITE
From: Tejun Heo @ 2018-07-18 11:47 UTC (permalink / raw)
To: axboe
Cc: michaelcallahan, newella, linux-block, linux-kernel, kernel-team,
linux-api, Tejun Heo, Theodore Ts'o, Jaegeuk Kim
In-Reply-To: <20180718114741.2580313-1-tj@kernel.org>
From: Michael Callahan <michaelcallahan@fb.com>
Add defines for STAT_READ and STAT_WRITE for indexing the partition
stat entries. This clarifies some fs/ code which has hardcoded 1 for
STAT_WRITE and will make it easier to extend the stats with additional
fields.
tj: Refreshed on top of v4.17.
Signed-off-by: Michael Callahan <michaelcallahan@fb.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
---
block/genhd.c | 16 ++++++++--------
block/partition-generic.c | 16 ++++++++--------
fs/ext4/super.c | 5 +++--
fs/ext4/sysfs.c | 6 ++++--
fs/f2fs/f2fs.h | 2 +-
fs/f2fs/super.c | 3 ++-
include/linux/blk_types.h | 7 +++++++
include/linux/genhd.h | 13 +++++++------
8 files changed, 40 insertions(+), 28 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index f1543a45e73b..0711a800d0d4 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1337,14 +1337,14 @@ static int diskstats_show(struct seq_file *seqf, void *v)
"%u %lu %lu %lu %u %u %u %u\n",
MAJOR(part_devt(hd)), MINOR(part_devt(hd)),
disk_name(gp, hd->partno, buf),
- part_stat_read(hd, ios[READ]),
- part_stat_read(hd, merges[READ]),
- part_stat_read(hd, sectors[READ]),
- jiffies_to_msecs(part_stat_read(hd, ticks[READ])),
- part_stat_read(hd, ios[WRITE]),
- part_stat_read(hd, merges[WRITE]),
- part_stat_read(hd, sectors[WRITE]),
- jiffies_to_msecs(part_stat_read(hd, ticks[WRITE])),
+ part_stat_read(hd, ios[STAT_READ]),
+ part_stat_read(hd, merges[STAT_READ]),
+ part_stat_read(hd, sectors[STAT_READ]),
+ jiffies_to_msecs(part_stat_read(hd, ticks[STAT_READ])),
+ part_stat_read(hd, ios[STAT_WRITE]),
+ part_stat_read(hd, merges[STAT_WRITE]),
+ part_stat_read(hd, sectors[STAT_WRITE]),
+ jiffies_to_msecs(part_stat_read(hd, ticks[STAT_WRITE])),
inflight[0],
jiffies_to_msecs(part_stat_read(hd, io_ticks)),
jiffies_to_msecs(part_stat_read(hd, time_in_queue))
diff --git a/block/partition-generic.c b/block/partition-generic.c
index 3dcfd4ec0e11..0ddb06722162 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -132,14 +132,14 @@ ssize_t part_stat_show(struct device *dev,
"%8lu %8lu %8llu %8u "
"%8u %8u %8u"
"\n",
- part_stat_read(p, ios[READ]),
- part_stat_read(p, merges[READ]),
- (unsigned long long)part_stat_read(p, sectors[READ]),
- jiffies_to_msecs(part_stat_read(p, ticks[READ])),
- part_stat_read(p, ios[WRITE]),
- part_stat_read(p, merges[WRITE]),
- (unsigned long long)part_stat_read(p, sectors[WRITE]),
- jiffies_to_msecs(part_stat_read(p, ticks[WRITE])),
+ part_stat_read(p, ios[STAT_READ]),
+ part_stat_read(p, merges[STAT_READ]),
+ (unsigned long long)part_stat_read(p, sectors[STAT_READ]),
+ jiffies_to_msecs(part_stat_read(p, ticks[STAT_READ])),
+ part_stat_read(p, ios[STAT_WRITE]),
+ part_stat_read(p, merges[STAT_WRITE]),
+ (unsigned long long)part_stat_read(p, sectors[STAT_WRITE]),
+ jiffies_to_msecs(part_stat_read(p, ticks[STAT_WRITE])),
inflight[0],
jiffies_to_msecs(part_stat_read(p, io_ticks)),
jiffies_to_msecs(part_stat_read(p, time_in_queue)));
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ba2396a7bd04..4b8aef989552 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3514,7 +3514,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
sbi->s_sb_block = sb_block;
if (sb->s_bdev->bd_part)
sbi->s_sectors_written_start =
- part_stat_read(sb->s_bdev->bd_part, sectors[1]);
+ part_stat_read(sb->s_bdev->bd_part, sectors[STAT_WRITE]);
/* Cleanup superblock name */
strreplace(sb->s_id, '/', '!');
@@ -4824,7 +4824,8 @@ static int ext4_commit_super(struct super_block *sb, int sync)
if (sb->s_bdev->bd_part)
es->s_kbytes_written =
cpu_to_le64(EXT4_SB(sb)->s_kbytes_written +
- ((part_stat_read(sb->s_bdev->bd_part, sectors[1]) -
+ ((part_stat_read(sb->s_bdev->bd_part,
+ sectors[STAT_WRITE]) -
EXT4_SB(sb)->s_sectors_written_start) >> 1));
else
es->s_kbytes_written =
diff --git a/fs/ext4/sysfs.c b/fs/ext4/sysfs.c
index f34da0bb8f17..2be9ad790017 100644
--- a/fs/ext4/sysfs.c
+++ b/fs/ext4/sysfs.c
@@ -56,7 +56,8 @@ static ssize_t session_write_kbytes_show(struct ext4_sb_info *sbi, char *buf)
if (!sb->s_bdev->bd_part)
return snprintf(buf, PAGE_SIZE, "0\n");
return snprintf(buf, PAGE_SIZE, "%lu\n",
- (part_stat_read(sb->s_bdev->bd_part, sectors[1]) -
+ (part_stat_read(sb->s_bdev->bd_part,
+ sectors[STAT_WRITE]) -
sbi->s_sectors_written_start) >> 1);
}
@@ -68,7 +69,8 @@ static ssize_t lifetime_write_kbytes_show(struct ext4_sb_info *sbi, char *buf)
return snprintf(buf, PAGE_SIZE, "0\n");
return snprintf(buf, PAGE_SIZE, "%llu\n",
(unsigned long long)(sbi->s_kbytes_written +
- ((part_stat_read(sb->s_bdev->bd_part, sectors[1]) -
+ ((part_stat_read(sb->s_bdev->bd_part,
+ sectors[STAT_WRITE]) -
EXT4_SB(sb)->s_sectors_written_start) >> 1)));
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4d8b1de83143..6799c3fc44e3 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1304,7 +1304,7 @@ static inline bool time_to_inject(struct f2fs_sb_info *sbi, int type)
* and the return value is in kbytes. s is of struct f2fs_sb_info.
*/
#define BD_PART_WRITTEN(s) \
-(((u64)part_stat_read((s)->sb->s_bdev->bd_part, sectors[1]) - \
+(((u64)part_stat_read((s)->sb->s_bdev->bd_part, sectors[STAT_WRITE]) - \
(s)->sectors_written_start) >> 1)
static inline void f2fs_update_time(struct f2fs_sb_info *sbi, int type)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 3995e926ba3a..17bcff789c08 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2882,7 +2882,8 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
/* For write statistics */
if (sb->s_bdev->bd_part)
sbi->sectors_written_start =
- (u64)part_stat_read(sb->s_bdev->bd_part, sectors[1]);
+ (u64)part_stat_read(sb->s_bdev->bd_part,
+ sectors[STAT_WRITE]);
/* Read accumulated write IO statistics if exists */
seg_i = CURSEG_I(sbi, CURSEG_HOT_NODE);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index e13449a379a1..d2b44de56bc1 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -357,6 +357,13 @@ enum req_flag_bits {
#define REQ_NOMERGE_FLAGS \
(REQ_NOMERGE | REQ_PREFLUSH | REQ_FUA)
+enum stat_group {
+ STAT_READ,
+ STAT_WRITE,
+
+ NR_STAT_GROUPS
+};
+
#define bio_op(bio) \
((bio)->bi_opf & REQ_OP_MASK)
#define req_op(req) \
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 19f36fa10995..a75445446974 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -16,6 +16,7 @@
#include <linux/slab.h>
#include <linux/percpu-refcount.h>
#include <linux/uuid.h>
+#include <linux/blk_types.h>
#ifdef CONFIG_BLOCK
@@ -82,10 +83,10 @@ struct partition {
} __attribute__((packed));
struct disk_stats {
- unsigned long sectors[2]; /* READs and WRITEs */
- unsigned long ios[2];
- unsigned long merges[2];
- unsigned long ticks[2];
+ unsigned long sectors[NR_STAT_GROUPS];
+ unsigned long ios[NR_STAT_GROUPS];
+ unsigned long merges[NR_STAT_GROUPS];
+ unsigned long ticks[NR_STAT_GROUPS];
unsigned long io_ticks;
unsigned long time_in_queue;
};
@@ -354,8 +355,8 @@ static inline void free_part_stats(struct hd_struct *part)
#endif /* CONFIG_SMP */
#define part_stat_read_accum(part, field) \
- (part_stat_read(part, field[0]) + \
- part_stat_read(part, field[1]))
+ (part_stat_read(part, field[STAT_READ]) + \
+ part_stat_read(part, field[STAT_WRITE]))
#define part_stat_add(cpu, part, field, addnd) do { \
__part_stat_add((cpu), (part), field, addnd); \
--
2.17.1
^ permalink raw reply related
* [PATCH 2/6] block: Add part_stat_read_accum to read across field entries.
From: Tejun Heo @ 2018-07-18 11:47 UTC (permalink / raw)
To: axboe
Cc: michaelcallahan, newella, linux-block, linux-kernel, kernel-team,
linux-api, Tejun Heo
In-Reply-To: <20180718114741.2580313-1-tj@kernel.org>
From: Michael Callahan <michaelcallahan@fb.com>
Add a part_stat_read_accum macro to genhd.h to read and sum across
field entries. For example to sum up the number read and write
sectors completed. In addition to being ar reasonable cleanup by
itself this will make it easier to add new stat fields in the future.
tj: Refreshed on top of v4.17.
Signed-off-by: Michael Callahan <michaelcallahan@fb.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/block/drbd/drbd_receiver.c | 3 +--
drivers/block/drbd/drbd_worker.c | 4 +---
drivers/md/md.c | 3 +--
include/linux/genhd.h | 4 ++++
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index a36a30795c43..75f6b47169e6 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -2674,8 +2674,7 @@ bool drbd_rs_c_min_rate_throttle(struct drbd_device *device)
if (c_min_rate == 0)
return false;
- curr_events = (int)part_stat_read(&disk->part0, sectors[0]) +
- (int)part_stat_read(&disk->part0, sectors[1]) -
+ curr_events = (int)part_stat_read_accum(&disk->part0, sectors) -
atomic_read(&device->rs_sect_ev);
if (atomic_read(&device->ap_actlog_cnt)
diff --git a/drivers/block/drbd/drbd_worker.c b/drivers/block/drbd/drbd_worker.c
index 5e793dd7adfb..b8f77e83d456 100644
--- a/drivers/block/drbd/drbd_worker.c
+++ b/drivers/block/drbd/drbd_worker.c
@@ -1690,9 +1690,7 @@ void drbd_rs_controller_reset(struct drbd_device *device)
atomic_set(&device->rs_sect_in, 0);
atomic_set(&device->rs_sect_ev, 0);
device->rs_in_flight = 0;
- device->rs_last_events =
- (int)part_stat_read(&disk->part0, sectors[0]) +
- (int)part_stat_read(&disk->part0, sectors[1]);
+ device->rs_last_events = (int)part_stat_read_accum(&disk->part0, sectors);
/* Updating the RCU protected object in place is necessary since
this function gets called from atomic context.
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 994aed2f9dff..dabe36723d60 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8046,8 +8046,7 @@ static int is_mddev_idle(struct mddev *mddev, int init)
rcu_read_lock();
rdev_for_each_rcu(rdev, mddev) {
struct gendisk *disk = rdev->bdev->bd_contains->bd_disk;
- curr_events = (int)part_stat_read(&disk->part0, sectors[0]) +
- (int)part_stat_read(&disk->part0, sectors[1]) -
+ curr_events = (int)part_stat_read_accum(&disk->part0, sectors) -
atomic_read(&disk->sync_io);
/* sync IO will cause sync_io to increase before the disk_stats
* as sync_io is counted when a request starts, and
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 6cb8a5789668..19f36fa10995 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -353,6 +353,10 @@ static inline void free_part_stats(struct hd_struct *part)
#endif /* CONFIG_SMP */
+#define part_stat_read_accum(part, field) \
+ (part_stat_read(part, field[0]) + \
+ part_stat_read(part, field[1]))
+
#define part_stat_add(cpu, part, field, addnd) do { \
__part_stat_add((cpu), (part), field, addnd); \
if ((part)->partno) \
--
2.17.1
^ 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