> > On Mon 29-01-24 20:30:34, Amir Goldstein wrote: > > > On Mon, Dec 18, 2023 at 5:53 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > In the HttpDirFS HSM demo, I used FAN_OPEN_PERM on a mount mark > > > > to deny open of file during the short time that it's content is being > > > > punched out. > > > > It is quite complicated to explain, but I only used it for denying access, > > > > not to fill content and not to write anything to filesystem. > > > > It's worth noting that returning EBUSY in that case would be more meaningful > > > > to users. > > > > > > > > That's one case in favor of allowing FAN_DENY_ERRNO for FAN_OPEN_PERM, > > > > but mainly I do not have a proof that people will not need it. > > > > > > > > OTOH, I am a bit concerned that this will encourage developer to use > > > > FAN_OPEN_PERM as a trigger to filling file content and then we are back to > > > > deadlock risk zone. > > > > > > > > Not sure which way to go. > > > > > > > > Anyway, I think we agree that there is no reason to merge FAN_DENY_ERRNO > > > > before FAN_PRE_* events, so we can continue this discussion later when > > > > I post FAN_PRE_* patches - not for this cycle. > > > > > > I started to prepare the pre-content events patches for posting and got back > > > to this one as well. > > > > > > Since we had this discussion I have learned of another use case that > > > requires filling file content in FAN_OPEN_PERM hook, FAN_OPEN_EXEC_PERM > > > to be exact. > > > > > > The reason is that unless an executable content is filled at execve() time, > > > there is no other opportunity to fill its content without getting -ETXTBSY. > > > > Yes, I've been scratching my head over this usecase for a few days. I was > > thinking whether we could somehow fill in executable (and executed) files on > > access but it all seemed too hacky so I agree that we probably have to fill > > them in on open. > > > > Normally, I think there will not be a really huge executable(?) > If there were huge executables, they would have likely been broken down > into smaller loadable libraries which should allow more granular > content filling, > but I guess there will always be worst case exceptions. > > > > So to keep things more flexible, I decided to add -ETXTBSY to the > > > allowed errors with FAN_DENY_ERRNO() and to decided to allow > > > FAN_DENY_ERRNO() with all permission events. > > > > > > To keep FAN_DENY_ERRNO() a bit more focused on HSM, I have > > > added a limitation that FAN_DENY_ERRNO() is allowed only for > > > FAN_CLASS_PRE_CONTENT groups. > > > > I have no problem with adding -ETXTBSY to the set of allowed errors. That > > makes sense. Adding FAN_DENY_ERRNO() to all permission events in > > FAN_CLASS_PRE_CONTENT groups - OK, > > done that. > > I am still not very happy about FAN_OPEN_PERM being part of HSM > event family when I know that O_TRUCT and O_CREAT call this hook > with sb writers held. > > The irony, is that there is no chance that O_TRUNC will require filling > content, same if the file is actually being created by O_CREAT, so the > cases where sb writers is actually needed and the case where content > filling is needed do not overlap, but I cannot figure out how to get those > cases out of the HSM risk zone. Ideas? > Jan, I wanted to run an idea by you. I like your idea to start a clean slate with FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID and it would be nice if we could restrict this HSM to use pre-content events, which is why I was not happy about allowing FAN_DENY_ERRNO() for the legacy FAN_OPEN*_PERM events, especially with the known deadlocks. Since we already know that we need to generate FAN_PRE_ACCESS(offset,length) for read-only mmap() and FAN_PRE_MODIFY(offset,length) for writable mmap(), we could treat uselib() and execve() the same way and generate FAN_PRE_ACCESS(0,i_size) as if the file was mmaped. My idea is to generate an event FAN_PRE_MODIFY(0,0) for an open for write *after* file was truncated and FAN_PRE_ACCESS(0,0) for open O_RDONLY. Possibly also FAN_PRE_*(offset,0) events for llseek(). I've already pushed a POC to fan_pre_content branch [1]. Only sanity tested that nothing else is broken. I still need to add the mmap hooks and test the new events. With this, HSM will have appropriate hooks to fill executable and library on first access and also fill arbitrary files on open including the knowledge if the file was opened for write. Thoughts? Thanks, Amir. [1] https://github.com/amir73il/linux/commits/fan_pre_content
