On 22.06.2020 15:11, Jiri Olsa wrote: > On Mon, Jun 22, 2020 at 01:50:03PM +0300, Alexey Budankov wrote: >> >> On 22.06.2020 13:21, Jiri Olsa wrote: >>> On Mon, Jun 22, 2020 at 12:47:19PM +0300, Alexey Budankov wrote: >>> >>> SNIP >>> >>>>>>>>>> fdarray__del(array, fdkey); >>>>>>>>> >>>>>>>>> I think there's solution without having filterable type, >>>>>>>>> I'm not sure why you think this is needed >>>>>>>>> >>>>>>>>> I'm busy with other things this week, but I think I can >>>>>>>>> come up with some patch early next week if needed >>>>>>>> >>>>>>>> Friendly reminder. >>>>>>> >>>>>>> hm? I believe we discussed this in here: >>>>>>> https://lore.kernel.org/lkml/20200609145611.GI1558310@krava/ >>>>>> >>>>>> Do you want it to be implemented like in the patch posted by the link? >>>>> >>>>> no idea.. looking for good solution ;-) >>>>> >>>>> how about switching completely to epoll? I tried and it >>>>> does not look that bad >>>> >>>> Well, epoll() is perhaps possible but why does it want switching to >>>> epoll()? >>>> What are the benefits and/or specific task being solved by this switch? >>> >>> epoll change fixes the same issues as the patch you took in v8 >>> >>> on top of it it's not a hack and wil make polling more user >>> friendly because of the clear interface >> >> Clear. The opposite thing is /proc/sys/fs/epoll/max_user_watches limit that >> will affect Perf tool usage additionally to the current process limit on >> a number of simultaneously open file descriptors (ulimit -n). So move to >> epoll() will impose one limit what can affect Perf tool scalability. > > hum, I dont think this will be a problem: > > Allowing top 4% of low memory (per user) to be allocated in epoll watches, > we have: > > LOMEM MAX_WATCHES (per user) > 512MB ~178000 > 1GB ~356000 > 2GB ~712000 > > my laptop has 19841945 allowed watches per user > >> >>> >>>> >>>>> >>>>> there might be some loose ends (interface change), but >>>>> I think this would solve our problems with fdarray >>>> >>>> Your first patch accomodated in v8 actually avoids fds typing >>>> and solves pos (=fdarray__add()) staleness issue with fdarray. >>> >>> yea, it was a change meant for discussion (which never happened), >>> and I considered it to be more a hack than a solution >>> >>> I suppose we can live with that for a while, but I'd like to >>> have clean solution for polling as well >> >> I wouldn't treat it as a hack but more as a fix because returned >> pos is now a part of interface that can be safely used in callers. >> Can we go with this fix for the patch set? > > apart from this one I still have a problem with that stat factoring > having 1 complicated function deal with both fork and no fork processing, > which I already commented on, but you ignored ;-)
Not an issue at all, lets split that func, dispatch_events() I suppose, as you see it. > > I'll try to go through that once more, and post some comments > > jirka > Thanks, Alexey