Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1719371 > unrolled thread
| Started by | Pavel Machek <pavel@ucw.cz> |
|---|---|
| First post | 2017-08-24 18:10 +0200 |
| Last post | 2017-08-31 11:20 +0200 |
| Articles | 11 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag Pavel Machek <pavel@ucw.cz> - 2017-08-24 18:10 +0200
Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag Michal Hocko <mhocko@kernel.org> - 2017-08-25 08:40 +0200
Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag Pavel Machek <pavel@ucw.cz> - 2017-08-25 09:30 +0200
Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag Michal Hocko <mhocko@kernel.org> - 2017-08-25 10:10 +0200
Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag Pavel Machek <pavel@ucw.cz> - 2017-08-25 23:40 +0200
Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag NeilBrown <neilb@suse.com> - 2017-08-26 06:20 +0200
Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag Michal Hocko <mhocko@kernel.org> - 2017-08-28 14:40 +0200
Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag Pavel Machek <pavel@ucw.cz> - 2017-08-31 11:10 +0200
Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag Mel Gorman <mgorman@suse.de> - 2017-08-31 11:30 +0200
Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag Michal Hocko <mhocko@kernel.org> - 2017-08-28 14:40 +0200
Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag Pavel Machek <pavel@ucw.cz> - 2017-08-31 11:20 +0200
| From | Pavel Machek <pavel@ucw.cz> |
|---|---|
| Date | 2017-08-24 18:10 +0200 |
| Subject | Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag |
| Message-ID | <ui2RI-kG-41@gated-at.bofh.it> |
Hi!
> From: Michal Hocko <mhocko@suse.com>
>
> GFP_TEMPORARY has been introduced by e12ba74d8ff3 ("Group short-lived
> and reclaimable kernel allocations") along with __GFP_RECLAIMABLE. It's
> primary motivation was to allow users to tell that an allocation is
> short lived and so the allocator can try to place such allocations close
> together and prevent long term fragmentation. As much as this sounds
> like a reasonable semantic it becomes much less clear when to use the
> highlevel GFP_TEMPORARY allocation flag. How long is temporary? Can
> the context holding that memory sleep? Can it take locks? It seems
> there is no good answer for those questions.
>
> The current implementation of GFP_TEMPORARY is basically
> GFP_KERNEL | __GFP_RECLAIMABLE which in itself is tricky because
> basically none of the existing caller provide a way to reclaim the
> allocated memory. So this is rather misleading and hard to evaluate for
> any benefits.
>
> I have checked some random users and none of them has added the flag
> with a specific justification. I suspect most of them just copied from
> other existing users and others just thought it might be a good idea
> to use without any measuring. This suggests that GFP_TEMPORARY just
> motivates for cargo cult usage without any reasoning.
>
> I believe that our gfp flags are quite complex already and especially
> those with highlevel semantic should be clearly defined to prevent from
> confusion and abuse. Therefore I propose dropping GFP_TEMPORARY and
> replace all existing users to simply use GFP_KERNEL. Please note that
> SLAB users with shrinkers will still get __GFP_RECLAIMABLE heuristic
> and so they will be placed properly for memory fragmentation prevention.
>
> I can see reasons we might want some gfp flag to reflect shorterm
> allocations but I propose starting from a clear semantic definition and
> only then add users with proper justification.
Dunno. < 1msec probably is temporary, 1 hour probably is not. If it causes
problems, can you just #define GFP_TEMPORARY GFP_KERNEL ? Treewide replace,
and then starting again goes not look attractive to me.
Pavel
[toc] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-08-25 08:40 +0200 |
| Message-ID | <uigrD-xp-11@gated-at.bofh.it> |
| In reply to | #1719371 |
On Wed 23-08-17 19:57:09, Pavel Machek wrote:
> Hi!
>
> > From: Michal Hocko <mhocko@suse.com>
> >
> > GFP_TEMPORARY has been introduced by e12ba74d8ff3 ("Group short-lived
> > and reclaimable kernel allocations") along with __GFP_RECLAIMABLE. It's
> > primary motivation was to allow users to tell that an allocation is
> > short lived and so the allocator can try to place such allocations close
> > together and prevent long term fragmentation. As much as this sounds
> > like a reasonable semantic it becomes much less clear when to use the
> > highlevel GFP_TEMPORARY allocation flag. How long is temporary? Can
> > the context holding that memory sleep? Can it take locks? It seems
> > there is no good answer for those questions.
> >
> > The current implementation of GFP_TEMPORARY is basically
> > GFP_KERNEL | __GFP_RECLAIMABLE which in itself is tricky because
> > basically none of the existing caller provide a way to reclaim the
> > allocated memory. So this is rather misleading and hard to evaluate for
> > any benefits.
> >
> > I have checked some random users and none of them has added the flag
> > with a specific justification. I suspect most of them just copied from
> > other existing users and others just thought it might be a good idea
> > to use without any measuring. This suggests that GFP_TEMPORARY just
> > motivates for cargo cult usage without any reasoning.
> >
> > I believe that our gfp flags are quite complex already and especially
> > those with highlevel semantic should be clearly defined to prevent from
> > confusion and abuse. Therefore I propose dropping GFP_TEMPORARY and
> > replace all existing users to simply use GFP_KERNEL. Please note that
> > SLAB users with shrinkers will still get __GFP_RECLAIMABLE heuristic
> > and so they will be placed properly for memory fragmentation prevention.
> >
> > I can see reasons we might want some gfp flag to reflect shorterm
> > allocations but I propose starting from a clear semantic definition and
> > only then add users with proper justification.
>
> Dunno. < 1msec probably is temporary, 1 hour probably is not. If it causes
> problems, can you just #define GFP_TEMPORARY GFP_KERNEL ? Treewide replace,
> and then starting again goes not look attractive to me.
I do not think we want a highlevel GFP_TEMPORARY without any meaning.
This just supports spreading the flag usage without a clear semantic
and it will lead to even bigger mess. Once we can actually define what
the flag means we can also add its users based on that new semantic.
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Pavel Machek <pavel@ucw.cz> |
|---|---|
| Date | 2017-08-25 09:30 +0200 |
| Message-ID | <uihe2-14M-3@gated-at.bofh.it> |
| In reply to | #1719774 |
[Multipart message — attachments visible in raw view] — view raw
On Fri 2017-08-25 08:35:46, Michal Hocko wrote:
> On Wed 23-08-17 19:57:09, Pavel Machek wrote:
> > Hi!
> >
> > > From: Michal Hocko <mhocko@suse.com>
> > >
> > > GFP_TEMPORARY has been introduced by e12ba74d8ff3 ("Group short-lived
> > > and reclaimable kernel allocations") along with __GFP_RECLAIMABLE. It's
> > > primary motivation was to allow users to tell that an allocation is
> > > short lived and so the allocator can try to place such allocations close
> > > together and prevent long term fragmentation. As much as this sounds
> > > like a reasonable semantic it becomes much less clear when to use the
> > > highlevel GFP_TEMPORARY allocation flag. How long is temporary? Can
> > > the context holding that memory sleep? Can it take locks? It seems
> > > there is no good answer for those questions.
> > >
> > > The current implementation of GFP_TEMPORARY is basically
> > > GFP_KERNEL | __GFP_RECLAIMABLE which in itself is tricky because
> > > basically none of the existing caller provide a way to reclaim the
> > > allocated memory. So this is rather misleading and hard to evaluate for
> > > any benefits.
> > >
> > > I have checked some random users and none of them has added the flag
> > > with a specific justification. I suspect most of them just copied from
> > > other existing users and others just thought it might be a good idea
> > > to use without any measuring. This suggests that GFP_TEMPORARY just
> > > motivates for cargo cult usage without any reasoning.
> > >
> > > I believe that our gfp flags are quite complex already and especially
> > > those with highlevel semantic should be clearly defined to prevent from
> > > confusion and abuse. Therefore I propose dropping GFP_TEMPORARY and
> > > replace all existing users to simply use GFP_KERNEL. Please note that
> > > SLAB users with shrinkers will still get __GFP_RECLAIMABLE heuristic
> > > and so they will be placed properly for memory fragmentation prevention.
> > >
> > > I can see reasons we might want some gfp flag to reflect shorterm
> > > allocations but I propose starting from a clear semantic definition and
> > > only then add users with proper justification.
> >
> > Dunno. < 1msec probably is temporary, 1 hour probably is not. If it causes
> > problems, can you just #define GFP_TEMPORARY GFP_KERNEL ? Treewide replace,
> > and then starting again goes not look attractive to me.
>
> I do not think we want a highlevel GFP_TEMPORARY without any meaning.
> This just supports spreading the flag usage without a clear semantic
> and it will lead to even bigger mess. Once we can actually define what
> the flag means we can also add its users based on that new semantic.
It has real meaning.
You can define more exact meaning, and then adjust the usage. But
there's no need to do treewide replacement...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-08-25 10:10 +0200 |
| Message-ID | <uihQJ-1wL-5@gated-at.bofh.it> |
| In reply to | #1719797 |
On Fri 25-08-17 09:28:19, Pavel Machek wrote: > On Fri 2017-08-25 08:35:46, Michal Hocko wrote: > > On Wed 23-08-17 19:57:09, Pavel Machek wrote: [...] > > > Dunno. < 1msec probably is temporary, 1 hour probably is not. If it causes > > > problems, can you just #define GFP_TEMPORARY GFP_KERNEL ? Treewide replace, > > > and then starting again goes not look attractive to me. > > > > I do not think we want a highlevel GFP_TEMPORARY without any meaning. > > This just supports spreading the flag usage without a clear semantic > > and it will lead to even bigger mess. Once we can actually define what > > the flag means we can also add its users based on that new semantic. > > It has real meaning. Which is? > You can define more exact meaning, and then adjust the usage. But > there's no need to do treewide replacement... I have checked most of them and except for the initially added onces the large portion where added without a good reasons or even break an intuitive meaning by taking locks. Seriously, if we need a short term semantic it should be clearly defined first. Is there any specific case why you think this patch is in a wrong direction? E.g. a measurable regression? -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Pavel Machek <pavel@ucw.cz> |
|---|---|
| Date | 2017-08-25 23:40 +0200 |
| Message-ID | <uiuuB-Rx-13@gated-at.bofh.it> |
| In reply to | #1719813 |
[Multipart message — attachments visible in raw view] — view raw
On Fri 2017-08-25 10:04:42, Michal Hocko wrote: > On Fri 25-08-17 09:28:19, Pavel Machek wrote: > > On Fri 2017-08-25 08:35:46, Michal Hocko wrote: > > > On Wed 23-08-17 19:57:09, Pavel Machek wrote: > [...] > > > > Dunno. < 1msec probably is temporary, 1 hour probably is not. If it causes > > > > problems, can you just #define GFP_TEMPORARY GFP_KERNEL ? Treewide replace, > > > > and then starting again goes not look attractive to me. > > > > > > I do not think we want a highlevel GFP_TEMPORARY without any meaning. > > > This just supports spreading the flag usage without a clear semantic > > > and it will lead to even bigger mess. Once we can actually define what > > > the flag means we can also add its users based on that new semantic. > > > > It has real meaning. > > Which is? "This allocation is temporary. It lasts milliseconds, not hours." > > You can define more exact meaning, and then adjust the usage. But > > there's no need to do treewide replacement... > > I have checked most of them and except for the initially added onces the > large portion where added without a good reasons or even break an > intuitive meaning by taking locks. I don't see it. kmalloc() itself takes locks. Of course everyone takes locks. I don't think that's intuitive meaning. > Seriously, if we need a short term semantic it should be clearly defined > first. "milliseconds, not hours." > Is there any specific case why you think this patch is in a wrong > direction? E.g. a measurable regression? Not playing that game. You should argue why it is improvement. And I don't believe you did. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[toc] | [prev] | [next] | [standalone]
| From | NeilBrown <neilb@suse.com> |
|---|---|
| Date | 2017-08-26 06:20 +0200 |
| Message-ID | <uiAJH-4TJ-3@gated-at.bofh.it> |
| In reply to | #1720424 |
[Multipart message — attachments visible in raw view] — view raw
On Fri, Aug 25 2017, Pavel Machek wrote: > On Fri 2017-08-25 10:04:42, Michal Hocko wrote: >> On Fri 25-08-17 09:28:19, Pavel Machek wrote: >> > On Fri 2017-08-25 08:35:46, Michal Hocko wrote: >> > > On Wed 23-08-17 19:57:09, Pavel Machek wrote: >> [...] >> > > > Dunno. < 1msec probably is temporary, 1 hour probably is not. If it causes >> > > > problems, can you just #define GFP_TEMPORARY GFP_KERNEL ? Treewide replace, >> > > > and then starting again goes not look attractive to me. >> > > >> > > I do not think we want a highlevel GFP_TEMPORARY without any meaning. >> > > This just supports spreading the flag usage without a clear semantic >> > > and it will lead to even bigger mess. Once we can actually define what >> > > the flag means we can also add its users based on that new semantic. >> > >> > It has real meaning. >> >> Which is? > > "This allocation is temporary. It lasts milliseconds, not hours." It isn't sufficient to give a rule for when GFP_TEMPORARY will be used, you also need to explain (at least in general terms) how the information will be used. Also you need to give guidelines on whether the flag should be set for allocation that will last seconds or minutes. If we have a flag that doesn't have a well defined meaning that actually affects behavior, it will not be used consistently, and if we ever change exactly how it behaves we can expect things to break. So it is better not to have a flag, than to have a poorly defined flag. My current thoughts is that the important criteria is not how long the allocation will be used for, but whether it is reclaimable. Allocations that will only last 5 msecs are reclaimable by calling "usleep(5000)". Other allocations might be reclaimable in other ways. Allocations that are not reclaimable may well be directed to a more restricted pool of memory, and might be more likely to fail. If we grew a strong "reclaimable" concept, this 'temporary' concept that you want to hold on to would become a burden. NeilBrown
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-08-28 14:40 +0200 |
| Message-ID | <ujruF-5AC-19@gated-at.bofh.it> |
| In reply to | #1720533 |
On Sat 26-08-17 14:11:33, NeilBrown wrote: > On Fri, Aug 25 2017, Pavel Machek wrote: > > > On Fri 2017-08-25 10:04:42, Michal Hocko wrote: > >> On Fri 25-08-17 09:28:19, Pavel Machek wrote: > >> > On Fri 2017-08-25 08:35:46, Michal Hocko wrote: > >> > > On Wed 23-08-17 19:57:09, Pavel Machek wrote: > >> [...] > >> > > > Dunno. < 1msec probably is temporary, 1 hour probably is not. If it causes > >> > > > problems, can you just #define GFP_TEMPORARY GFP_KERNEL ? Treewide replace, > >> > > > and then starting again goes not look attractive to me. > >> > > > >> > > I do not think we want a highlevel GFP_TEMPORARY without any meaning. > >> > > This just supports spreading the flag usage without a clear semantic > >> > > and it will lead to even bigger mess. Once we can actually define what > >> > > the flag means we can also add its users based on that new semantic. > >> > > >> > It has real meaning. > >> > >> Which is? > > > > "This allocation is temporary. It lasts milliseconds, not hours." > > It isn't sufficient to give a rule for when GFP_TEMPORARY will be used, > you also need to explain (at least in general terms) how the information > will be used. Also you need to give guidelines on whether the flag > should be set for allocation that will last seconds or minutes. > > If we have a flag that doesn't have a well defined meaning that actually > affects behavior, it will not be used consistently, and if we ever > change exactly how it behaves we can expect things to break. So it is > better not to have a flag, than to have a poorly defined flag. Absolutely agreed! > My current thoughts is that the important criteria is not how long the > allocation will be used for, but whether it is reclaimable. Allocations > that will only last 5 msecs are reclaimable by calling "usleep(5000)". > Other allocations might be reclaimable in other ways. Allocations that > are not reclaimable may well be directed to a more restricted pool of > memory, and might be more likely to fail. If we grew a strong > "reclaimable" concept, this 'temporary' concept that you want to hold on > to would become a burden. ... and here again. The whole motivation for the flag was to gather these objects together and reduce chances of internal fragmentation due to long lived objects mixed with short term ones. Without an explicit way to reclaim those objects or having a clear checkpoint to wait for it is not really helping us to reach desired outcome (less fragmented memory). -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Pavel Machek <pavel@ucw.cz> |
|---|---|
| Date | 2017-08-31 11:10 +0200 |
| Message-ID | <uktE8-3Hh-61@gated-at.bofh.it> |
| In reply to | #1721576 |
[Multipart message — attachments visible in raw view] — view raw
Hi! > > > "This allocation is temporary. It lasts milliseconds, not hours." > > > > It isn't sufficient to give a rule for when GFP_TEMPORARY will be used, > > you also need to explain (at least in general terms) how the information > > will be used. Also you need to give guidelines on whether the flag > > should be set for allocation that will last seconds or minutes. > > > > If we have a flag that doesn't have a well defined meaning that actually > > affects behavior, it will not be used consistently, and if we ever > > change exactly how it behaves we can expect things to break. So it is > > better not to have a flag, than to have a poorly defined flag. > > Absolutely agreed! > > > My current thoughts is that the important criteria is not how long the > > allocation will be used for, but whether it is reclaimable. Allocations > > that will only last 5 msecs are reclaimable by calling "usleep(5000)". > > Other allocations might be reclaimable in other ways. Allocations that > > are not reclaimable may well be directed to a more restricted pool of > > memory, and might be more likely to fail. If we grew a strong > > "reclaimable" concept, this 'temporary' concept that you want to hold on > > to would become a burden. > > ... and here again. The whole motivation for the flag was to gather > these objects together and reduce chances of internal fragmentation > due to long lived objects mixed with short term ones. Without an > explicit way to reclaim those objects or having a clear checkpoint to > wait for it is not really helping us to reach desired outcome (less > fragmented memory). Really? If you group allocations that last << 1 second, and ones that last >> 1 second, I'm pretty sure it reduces fragmentation... "reclaimable" or not. Fragmentation is just statistical property, so getting it "mostly right" helps... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[toc] | [prev] | [next] | [standalone]
| From | Mel Gorman <mgorman@suse.de> |
|---|---|
| Date | 2017-08-31 11:30 +0200 |
| Message-ID | <uktXs-3NS-13@gated-at.bofh.it> |
| In reply to | #1724019 |
On Thu, Aug 31, 2017 at 11:07:22AM +0200, Pavel Machek wrote: > > > > "This allocation is temporary. It lasts milliseconds, not hours." > > > > > > It isn't sufficient to give a rule for when GFP_TEMPORARY will be used, > > > you also need to explain (at least in general terms) how the information > > > will be used. Also you need to give guidelines on whether the flag > > > should be set for allocation that will last seconds or minutes. > > > > > > If we have a flag that doesn't have a well defined meaning that actually > > > affects behavior, it will not be used consistently, and if we ever > > > change exactly how it behaves we can expect things to break. So it is > > > better not to have a flag, than to have a poorly defined flag. > > > > Absolutely agreed! > > > > > My current thoughts is that the important criteria is not how long the > > > allocation will be used for, but whether it is reclaimable. Allocations > > > that will only last 5 msecs are reclaimable by calling "usleep(5000)". > > > Other allocations might be reclaimable in other ways. Allocations that > > > are not reclaimable may well be directed to a more restricted pool of > > > memory, and might be more likely to fail. If we grew a strong > > > "reclaimable" concept, this 'temporary' concept that you want to hold on > > > to would become a burden. > > > > ... and here again. The whole motivation for the flag was to gather > > these objects together and reduce chances of internal fragmentation > > due to long lived objects mixed with short term ones. Without an > > explicit way to reclaim those objects or having a clear checkpoint to > > wait for it is not really helping us to reach desired outcome (less > > fragmented memory). > > Really? > > If you group allocations that last << 1 second, and ones that last >> > 1 second, I'm pretty sure it reduces fragmentation... "reclaimable" or > not. > If this was always done reliably then sure, it makes sense. At the time it was introduced by me, proc was used to relay large amounts of information to userspace. The patch had a noticable impact but on limited memory, 32-bit and this proc relay was in use. In retrospect, it's possible that it was the monitoring itself that showed a "benefit" for the patch. If the flag is used incorrectly even once then the value is diminished and it can even cause harm (not as severe as misusing __GFP_MOVABLE but harmful nonetheless). It only has a benefit if there is a large source of temporary allocations that are long-lived enough to cause fragmentation during small intervals and to be honest, directly measuring that is extremely difficult. The benefit is too marginal, the potential for harm is high and Michal is right to remove it. -- Mel Gorman SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-08-28 14:40 +0200 |
| Message-ID | <ujruF-5AC-1@gated-at.bofh.it> |
| In reply to | #1720424 |
On Fri 25-08-17 23:39:36, Pavel Machek wrote: > On Fri 2017-08-25 10:04:42, Michal Hocko wrote: > > On Fri 25-08-17 09:28:19, Pavel Machek wrote: > > > On Fri 2017-08-25 08:35:46, Michal Hocko wrote: > > > > On Wed 23-08-17 19:57:09, Pavel Machek wrote: > > [...] > > > > > Dunno. < 1msec probably is temporary, 1 hour probably is not. If it causes > > > > > problems, can you just #define GFP_TEMPORARY GFP_KERNEL ? Treewide replace, > > > > > and then starting again goes not look attractive to me. > > > > > > > > I do not think we want a highlevel GFP_TEMPORARY without any meaning. > > > > This just supports spreading the flag usage without a clear semantic > > > > and it will lead to even bigger mess. Once we can actually define what > > > > the flag means we can also add its users based on that new semantic. > > > > > > It has real meaning. > > > > Which is? > > "This allocation is temporary. It lasts milliseconds, not hours." And why would such a semantic make any sense what so ever? We certainly do not try to wait for a pinned memory for $TIMEOUT when somebody really needs a larger memory block and there is a temporary allocation standing in the way. We simply do not know that an object is a temporary one. > > > You can define more exact meaning, and then adjust the usage. But > > > there's no need to do treewide replacement... > > > > I have checked most of them and except for the initially added onces the > > large portion where added without a good reasons or even break an > > intuitive meaning by taking locks. > > I don't see it. kmalloc() itself takes locks. Of course everyone takes > locks. I don't think that's intuitive meaning. I was talking about users of the flag. I have seen some to take a lock right after they allocated GFP_TEMPORARY object. > > Seriously, if we need a short term semantic it should be clearly defined > > first. > > "milliseconds, not hours." > > > Is there any specific case why you think this patch is in a wrong > > direction? E.g. a measurable regression? > > Not playing that game. You should argue why it is improvement. And I > don't believe you did. Please read the whole changelog where I was quite verbose about how the current flag is abused and how its semantic is weak and encourages a wrong usage pattern. Moreover it is not even clear whether it helps anything. I haven't seen any actual counter argument from you other than "milliseconds not hours" without actually explaining how that would be useful for any decisions done in the core MM layer. -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Pavel Machek <pavel@ucw.cz> |
|---|---|
| Date | 2017-08-31 11:20 +0200 |
| Message-ID | <uktNN-3Kz-31@gated-at.bofh.it> |
| In reply to | #1721573 |
[Multipart message — attachments visible in raw view] — view raw
Hi! > > > > You can define more exact meaning, and then adjust the usage. But > > > > there's no need to do treewide replacement... > > > > > > I have checked most of them and except for the initially added onces the > > > large portion where added without a good reasons or even break an > > > intuitive meaning by taking locks. > > > > I don't see it. kmalloc() itself takes locks. Of course everyone takes > > locks. I don't think that's intuitive meaning. > > I was talking about users of the flag. I have seen some to take a lock > right after they allocated GFP_TEMPORARY object. Yes, I'd expect people to take locks after allocating temporary objects. kmalloc itself takes locks. If the allocation is "usually" freed within miliseconds, that should be enough. > > > Seriously, if we need a short term semantic it should be clearly defined > > > first. > > > > "milliseconds, not hours." > > > > > Is there any specific case why you think this patch is in a wrong > > > direction? E.g. a measurable regression? > > > > Not playing that game. You should argue why it is improvement. And I > > don't believe you did. > > Please read the whole changelog where I was quite verbose about how the > current flag is abused and how its semantic is weak and encourages a > wrong usage pattern. Moreover it is not even clear whether it helps > anything. I haven't seen any actual counter argument from you other than > "milliseconds not hours" without actually explaining how that would be > useful for any decisions done in the core MM layer. Well, I find that argumentation insufficient for global search&replace. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web