On Tue 06-08-13 12:15:09, Tejun Heo wrote: > Hello, Michal. > > On Tue, Aug 06, 2013 at 05:58:04PM +0200, Michal Hocko wrote: > > I am objecting to moving the generic part of that code into memcg. The > > memcg part and the additional complexity (all the parsing and conditions > > for signalling) is already in the memcg code. > > But how is it generic if it's specific to memcg? How is it specific to memcg? The fact only memcg uses the interface doesn't imply it is memcg specific. > The practical > purpose here is making it clear that the interface is only used by > memcg and preventing any new usages from sprining up and the best way > to achieve that is making the code actually memcg-specific. There are other ways to achieve the same. E.g. not ack new usage of register callback users. We have done similar with other things like use_hierarchy... > It also helps cleaning up cftype in general. I'm not sure what you're > objecting to here. The cleanup is removing 2 callbacks with a cost of moving non-memcg specific code inside memcg. That is what I am objecting to. > > Such an interface would be really welcome but I would also ask how > > it would implement/allow context passing. E.g. how do we know which > > treshold has been reached? How do we find out the vmpressure level? Is > > the consumer supposed to do an additional action after it gets > > notification? > > Etc. > > Yeap, exactly and that's how it should have been from the beginning. > Attaching information to notification itself isn't a particularly good > design (anyone remembers rtsig?) if there's polling mechanism to > report the current state. There are pros and cons for both approaches and it should be discussed in a separate thread with a code to back all the claims. > It essentially amounts to duplicate delivery mechanisms for the same > information, which you usually don't want. Here, the inconvenience / > performance implications are negligible or even net-positive. Plain > file modified notification is way more familiar / conventional and > the overhead of an extra read call, which is highly unlikely to be > relevant given the expected frequency of the events we're talking > about, is small compared to the action of event delivery and context > switch. > > > Really that natural? So memcg should touch internals like cgroup dentry > > Functionally, it is completely specific to memcg at this point. It's > the only user and will stay the only user. > > > reference counting. You seem have forgotten all the hassles with > > cgroup_mutex, haven't you? > > Was the above sentence necessary? > > > No that part doesn't belong to memcg! You can discourage from new usage > > of this interface of course. > > Oh, if you're objecting to the details of the implementation, we of > course can clean it up. It should conceptually and functionally be > part of memcg and that is the guiding line we follow. Implementations > follow the concepts and functions, not the other way around. The > refcnt of course can be replaced with memcg css refcnting and we can > of course factor out dentry comparison in a prettier form. > > Compare it to the other way around - having event callbacks in cftype > and clearing code embedded in cgroup core destruction path when both > of which are completely irrelevant to all other controllers. Let's > clean up the implementation details and put things where they belong. > What's the excuse for not doing so when it's almost trivially doable? I will not repeat myself. We seem to disagree on where the code belongs. As I've said I will not ack this code, try to find somebody else who think it is a good idea. I do not see any added value. -- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>