On Mon, Dec 02, 2024 at 10:52:13AM +0000, Lorenzo Stoakes wrote: > On Fri, Nov 08, 2024 at 02:28:14PM +0000, Lorenzo Stoakes wrote: > > On Wed, Oct 30, 2024 at 04:37:37PM +0000, Lorenzo Stoakes wrote: > > > On Mon, Oct 28, 2024 at 04:06:07PM +0000, Lorenzo Stoakes wrote: > > > > I guess I'll try to adapt that and respin a v7 when I get a chance. > > > > > > Hm looking at this draft patch, it seems like a total rework of pidfd's > > > across the board right (now all pidfd's will need to be converted to > > > pid_fd)? Correct me if I'm wrong. > > > > > > If only for the signal case, it seems like overkill to define a whole > > > pid_fd and to use this CLASS() wrapper just for this one instance. > > > > > > If the intent is to convert _all_ pidfd's to use this type, it feels really > > > out of scope for this series and I think we'd probably instead want to go > > > off and do that as a separate series and put this on hold until that is > > > done. > > > > > > If instead you mean that we ought to do something like this just for the > > > signal case, it feels like it'd be quite a bit of extra abstraction just > > > used in this one case but nowhere else, I think if you did an abstraction > > > like this it would _have_ to be across the board right? > > > > > > I agree that the issue is with this one signal case that pins only the fd > > > (rather than this pid) where this 'pinning' doesn't _necessary_ mess around > > > with reference counts. > > > > > > So we definitely must address this, but the issue you had with the first > > > approach was that I think (correct me if I'm wrong) I was passing a pointer > > > to a struct fd which is not permitted right? > > > > > > Could we pass the struct fd by value to avoid this? I think we'd have to > > > unfortunately special-case this and probably duplicate some code which is a > > > pity as I liked the idea of abstracting everything to one place, but we can > > > obviously do that. > > > > > > So I guess to TL;DR it, the options are: > > > > > > 1. Implement pid_fd everywhere, in which case I will leave off on > > > this series and I guess, if I have time I could look at trying to > > > implement that or perhaps you'd prefer to? > > > > > > 2. We are good for the sake of this series to special-case a pidfd_to_pid() > > > implementation (used only by the pidfd_send_signal() syscall) > > > > > > 3. Something else, or I am misunderstanding your point :) > > > > > > Let me know how you want me to proceed on this as we're at v6 already and I > > > want to be _really_ sure I'm doing what you want here. > > > > > > Thanks! > > > > Hi Christian, > > > > Just a gentle nudge on this - as I need some guidance in order to know how > > to move the series forwards. > > > > Obviously no rush if your workload is high at the moment as this is pretty > > low priority, but just in case you missed it :) > > > > Thanks, Lorenzo > > Hi Christian, > > Just a ping on this now we're past the merge window and it's been over a > month. > > It'd be good to at least get a polite ack to indicate you're aware even if > you don't have the time to respond right now. > > If you'd prefer this series not to go ahead just let me know, but > unfortunately I really require your input to know how to move forward > otherwise I risk doing work that you might then reject. > Hey Lorenzo & Christian, what's the latest here? I see Christian has code suggestions at [1] which just needs to be addressed. Any thing else? I am hoping we can get this merged in the coming open window. [1] https://lore.kernel.org/linux-mm/20241202-wahrnehmen-mitten-e330cbd1eaf0@brauner/
