handle forget inline instead of in goroutine

[kahing]

when we are under memory pressure, or echo 3 > /proc/sys/vm/drop_caches,
kernel can send us many forget ops at the same time if we have lots of
inodes in memory. Running them all in goroutines can lead to even more
memory usage and eventually OOM.

[jacobsa]

There are some potential performance and deadlock issues that I'm trying to
think through here:

• fileSystemServer.handleOp calls into Connection.Reply, which eventually writes the response to the kernel if kernelResponse returns false. Fortunately it returns true for ForgetInodeOp. So we don't have to worry about a deadlock where the kernel waits to consume the response until we've pulled more off the incoming queue, but we're blocking on writing the response. So I think this is okay.

• The only other substantive thing that handleOp does is call FileSystem.ForgetInode. I worry that it may be an issue if this blocks on some other op coming in. But I can't think of any case where that would be reasonable, and indeed in the gcsfuse implementation at least the function wouldn't behave any different with a synchronous call.

So I'm happy with this overall. Could you please make a few changes though?

1. Add comments in the code you've changed that say why you're calling synchronously in this case. Something like "Special case: call in this goroutine for forget inode ops, which may come in a flurry from the kernel and are generally cheap for the file system to handle."

2. Fix the comment on NewFileSystemServer that's no longer true. Document that ForgetInode may be called synchronously, and should not depend on calls to other methods being received concurrently.

[kahing]

amended commit with comments

[jacobsa]

Thanks!

[zhjunqin]

@kahing

I'm not sure my issue has anything to do with this or not.

Could you help check #60 ?

Thanks a lot!