All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: willy@bombadil.infradead.org
Cc: willy@infradead.org, lsf-pc@lists.linux-foundation.org,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [ATTEND] many topics
Date: Thu, 19 Jan 2017 13:11:36 +0100	[thread overview]
Message-ID: <20170119121135.GR30786@dhcp22.suse.cz> (raw)
In-Reply-To: <20170119115243.GB22816@bombadil.infradead.org>

On Thu 19-01-17 03:52:43, willy@bombadil.infradead.org wrote:
> On Thu, Jan 19, 2017 at 12:33:17PM +0100, Michal Hocko wrote:
> > On Thu 19-01-17 03:05:13, willy@infradead.org wrote:
> > > Let me rephrase the topic ... Under what conditions should somebody use
> > > the GFP_TEMPORARY gfp_t?
> > 
> > Most users of slab (kmalloc) do not really have to care. Slab will add
> > __GFP_RECLAIMABLE to all reclaimable caches automagically AFAIR. The
> > remaining would have to implement some kind of shrinker to allow the
> > reclaim.
> 
> I seem to be not making myself clear.  Picture me writing a device driver.
> When should I use GFP_TEMPORARY?

I guess the original intention was to use this flag for allocations
which will be either freed shortly or they are reclaimable.
 
> > > Example usages that I have questions about:
> > > 
> > > 1. Is it permissible to call kmalloc(GFP_TEMPORARY), or is it only
> > > for alloc_pages?
> > 
> > kmalloc will use it internally as mentioned above.  I am not even sure
> > whether direct using of kmalloc(GFP_TEMPORARY) is ok.  I would have to
> > check the code but I guess it would be just wrong unless you know your
> > cache is reclaimable.
> 
> You're not using words that have any meaning to a device driver writer.
> Here's my code:
> 
> int foo_ioctl(..)
> {
> 	struct foo *foo = kmalloc(sizeof(*foo), GFP_TEMPORARY);
> }
> 
> Does this work?  If not, should it?  Or should slab be checking for
> this and calling WARN()?

I would have to check the code but I believe that this shouldn't be
harmful other than increase the fragmentation.

> > > I ask because if the slab allocator is unaware of
> > > GFP_TEMPORARY, then a non-GFP_TEMPORARY allocation may be placed in a
> > > page allocated with GFP_TEMPORARY and we've just made it meaningless.
> > > 
> > > 2. Is it permissible to sleep while holding a GFP_TEMPORARY allocation?
> > > eg, take a mutex, or wait_for_completion()?
> > 
> > Yes, GFP_TEMPORARY has ___GFP_DIRECT_RECLAIM set so this is by
> > definition sleepable allocation request.
> 
> Again, we're talking past each other.  Can foo_ioctl() sleep before
> releasing its GFP_TEMPORARY allocation, or will that make the memory
> allocator unhappy?

I do not think it would make the allocator unhappy as long as the sleep
is not for ever...

> > > 3. Can I make one GFP_TEMPORARY allocation, and then another one?
> > 
> > Not sure I understand. WHy would be a problem?
> 
> As you say above, GFP_TEMPORARY may sleep, so this is a variation on the "can I sleep while holding a GFP_TEMPORARY allocation" question.
> 
> > > 4. Should I disable preemption while holding a GFP_TEMPORARY allocation,
> > > or are we OK with a task being preempted?
> > 
> > no, it can sleep.
> > 
> > > 5. What about something even longer duration like allocating a kiocb?
> > > That might take an arbitrary length of time to be freed, but eventually
> > > the command will be timed out (eg 30 seconds for something that ends up
> > > going through SCSI).
> > 
> > I do not understand. The reclaimability of the object is in hands of the
> > respective shrinker...
> 
> There is no shrinker here.  This is about the object being "temporary",
> for some value of temporary.  I want to nail down what the MM is willing
> to tolerate in terms of length of time an object is allocated for.

>From my understanding MM will use the information for optimizing objects
placing and the longer the user will use that memory the worse this
optimization works. I do not think the (ab)use would be fatal...
 
> > > 6. Or shorter duration like doing a GFP_TEMPORARY allocation, then taking
> > > a spinlock, which *probably* isn't contended, but you never know.
> > > 
> > > 7. I can see it includes __GFP_WAIT so it's not suitable for using from
> > > interrupt context, but interrupt context might be the place which can
> > > benefit from it the most.  Or does GFP_ATOMIC's __GFP_HIGH also allow for
> > > allocation from the movable zone?  Should we have a GFP_TEMPORARY_ATOMIC?
> > 
> > This is where __GFP_RECLAIMABLE should be used as this is the core of
> > the functionality.
> 
> This response also doesn't make sense to me.

I meant to say that such an allocation can use __GFP_RECLAIMABLE | __GFP_NOWAIT.


-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-01-19 12:11 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-18  5:49 [ATTEND] many topics Matthew Wilcox
2017-01-18 10:13 ` [Lsf-pc] " Jan Kara
2017-01-18 11:26   ` willy
2017-01-18 13:32 ` Michal Hocko
2017-01-19 11:05   ` willy
2017-01-19 11:33     ` Michal Hocko
2017-01-19 11:52       ` willy
2017-01-19 12:11         ` Michal Hocko [this message]
2017-01-21  0:11           ` NeilBrown
2017-01-21 13:16             ` Theodore Ts'o
2017-01-22  4:45               ` NeilBrown
2017-01-23  6:05                 ` Matthew Wilcox
2017-01-23  6:30                   ` NeilBrown
2017-01-23  6:35                     ` Matthew Wilcox
2017-01-23 17:09                   ` Theodore Ts'o
2017-01-23 19:34                     ` NeilBrown
2017-01-25 14:36                       ` Vlastimil Babka
2017-01-25 20:36                         ` Matthew Wilcox
2017-01-25 21:15                           ` Vlastimil Babka
2017-01-25 23:19                         ` NeilBrown
2017-01-26  8:56                           ` Michal Hocko
2017-01-26 21:20                             ` NeilBrown
2017-01-27 13:12                               ` Michal Hocko

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20170119121135.GR30786@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lsf-pc@lists.linux-foundation.org \
    --cc=willy@bombadil.infradead.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

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

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