On Thu, Jan 19 2017, Michal Hocko wrote:
> On Thu 19-01-17 03:52:43, willy@xxxxxxxxxxxxxxxxxxxxxx wrote:
>> On Thu, Jan 19, 2017 at 12:33:17PM +0100, Michal Hocko wrote:
>> > On Thu 19-01-17 03:05:13, willy@xxxxxxxxxxxxx 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.
I would really like to see GFP_TEMPORARY described as a contract, rather
than in terms of implementation details.
What are the benefits of using it, and what are the costs?
For example, with GFP_NOFS, we know that the benefits are "no recursion
into the filesystem for reclaim" and hence no deadlocks. The costs are
that failure is more likely. So it is easy to know when to use it, and
it is easy to see if either side breaks the contract.
What are the benefits of GFP_TEMPORARY? Presumably it doesn't guarantee
success any more than GFP_KERNEL does, but maybe it is slightly less
likely to fail, and somewhat less likely to block for a long time?? But
without some sort of promise, I wonder why anyone would use the
flag. Is there a promise? Or is it just "you can be nice to the MM
layer by setting this flag sometimes". ???
And what, exactly, are the costs? How soon is "shortly". Below you say
"not forever" which very very different to "shortly", at least it is on
my calendar
I would like to suggest:
GFP_TEMPORARY should be used when the memory allocated will either be
freed, or will be placed in a reclaimable cache, before the process
which allocated it enters an TASK_INTERRUPTIBLE sleep or returns to
user-space. It allows access to memory which is usually reserved for
XXX and so can be expected to succeed more quickly during times of
high memory pressure.
Using GFP_TEMPORARY would then help make the code self-documenting and
might improve behaviour under memory pressure in some cases. It would
also be clear whether a particular was not correct, if a change in
behaviour of the MM would be consistent.
The rules given here might be more strict that necessary with the current
implementation, but they are clear and measurable. This gives room for
code to change in the future without breaking things.
NeilBrown
>
>> > > 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 from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: PGP signature
