The patch is tracked here.
http://review.whamcloud.com/#/c/10029/
Bobijam
Could you please review this suggested feature and provide feedback?
Thanks
Peter
I also noticed while looking at LU-137 that there is an ioctl in newer versions of ext4 EXT4_IOC_PRECACHE_EXTENTS which fetches only the file metadata from disk. This might be useful in conjunction with this patch to avoid the random seeking for the data, but avoid polluting cache with the data if the reads are going to be large. This would imply that we need several different levels of "advice" for the OSTs, like ADVICE_CACHE_METADATA, ADVICE_CACHE_DATA (or FADV_WILLNEED from fadvise()), ADVICE_UNCACHE (or FADV_DONTNEED from fadvise()).
For ADVICE_RANDOM, if the file size is small enough that the file can fit entirely into the client RAM or the objects can fit entirely into the OST(s) RAM then it could be mapped to ADVICE_CACHE_DATA. Otherwise it should disable readahead on the file/object. This would also be useful if there was a burst buffer device that could copy the file from the OSTs into SSD/NVRAM storage, but that doesn't exist yet.
For ADVICE_SEQUENTIAL it would make sense to increase the readahead window for the file on both the client and the backing OST(s).
Hi Andreas,
Thank you very much for the guidance. That is really interesting. I will add these hints when I get the chance to refresh the patch.
There could be different implemetations for each kind of advice/hint. So, in order to make it easier to review, I seperate the change into several patches. The first patch adds a framework and other patches add advices/hints support based on the framework.
Framework patch:
http://review.whamcloud.com/12458
Willneed patch:
http://review.whamcloud.com/#/c/10029/
The purpose of this patch wasn't clear to me. This patch may work under a few assumptions:
1. disk delay consumes most of time in the read sys call handling;
2. the readcache on the OFD size can hold readahead pages long enough so that they are still in cache when they are actually used.
I'm not sure if we can make these assumptions.
The problem to block us from implementing fadivse(2) is that there is no file system callback for this interface. If we can take the ioctl() solution as this patch suggested, we can alternatively implement an ioctl based fadvise(2)(which can be easily migrated once the file system callback of fadvise(2) is added into kernel). Therefore, WILLNEED will actually read ahead pages from OFD side.
How do you think?
Hi Jinshan,
We did a little bit benchmark with this patch for concept validation. If we prefetch data from disk to memory, we will be able to get a huge improvement for read performance, especially for small read. And even if we prefetch the data from disk to SSD (with the support from hardware APIs), the performance improvement is still significant. I am not sure, but I guess, comparing to pretching to memory, this patch might be more useful for hybrid storage drivers, because as you said, the memory size is really limited.
Using existing IOCTL framework is an interesting idea. But maybe, there are some other advice types (for example, some of the types that Andreas had suggested) which don't need to be sent to OSS/MDS side. IOCTL seems like a low level interfaces which controls ldiskfs/ZFS directly. Fadvise could be more upper-level and might trigger smart reactions from different levels of Lustre components. So, I think it is a little bit different with IOCTL interfaces. 
I had a conversation with Li Xi. It turned out that this is not exactly the same scenario of fadvise(), the goal of this work is not even to read WILLNEED pages into memory. The intention is to work on a specialized hardware where it has HDD on the OST and SSD as the cache of HDD, and it just prefetches the data from HDD to SSD so that the upcoming read can find the data in SSD. The data can be read by multiple clients therefore it may not be good to transfer those data to client's memory as fadvise() does.
In that case, it's really confusing to use terminologies of fadvise(). I would like to change the name to prefetch or something else.
Li Xi, please correct me if I got this wrong.
Hi Jinshan,
Yeah, the scenario of this work is different with traditional fadvise(). So it would be a good idea to use a different name to prevent confusion.
JInshan, I think "prefetch" is not necessarily correct either, since there may be a desire to do e.g. DONTNEED or NOREUSE to flush data from the cache. I don't mind "ladvise" as the name, since it is essentially "fadvise, but on the server and not the client". This may also be able to integrate into DSS into the future to give cache hints to the server.
Hi Andreas, If I could make a choice, I'd avoid ladvise() and any terminologies similar to fadvise(). Sooner or later, we're going to implement fadvise(2) for Lustre, and people will start to ask what's the difference between ladvise() and fadvise() 
Hi Jinshan,
Do you have any better idea about the name? I am fine with ladvise(). I guess the difference between fadvise() and this mechanism has to be explained anyway no matter which name we choose. The difference between names of ladvise() and fadvise() looks enough to alert people that they are similar but different. And names that has similar meaning, e.g. hint and intent, might conflicts with existing mechanisms too, which will be worse.
Li Xi (lixi@ddn.com) uploaded a new patch: http://review.whamcloud.com/13691
Subject: LU-4931 ladvise: Add feature of giving file access advices
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 0252db601d888f5923fd5a6dfec886c2a1c68038
I just read through the patch (and left a bunch of comments).
I think it leaves more questions than it answers.
e.g. you mention that "senior users" would be able to do this (to influence "dumb applications" I assume). But if you will walk this road - it opens all sorts of questions, like how sticky those advices are, how to get list of advices currently in effect on a file and in the filesystem, how to reset the advices so that they are no longer in effect and such.
The very important question of why did not you try to talk to upstream kernel people to see if they would be willing to add a callback in fadvise system call to call into th filesystem? If they are willing to do this - your job is suddenly much simplified as a lot of 3rd party apps that currently use posix_fadvise would start magically working and we won't have one more API to think about.
Have you considered that there's no sense to send any advises to the servers all by themself? Why not cache the advise information on the client - not only would you be able to glance information about this from the file descriptor (even if the kernel guys don't agree to insert an FS callback) for compat reasons, there's one less RPC, and then you can only send relevant advices with every IO too, and refresh server idea of your wishes every time should the server forget (due to failover/recovery or memory pressure or countless other reasons). I see your example of "migrate data to other tier of storage" potentially by a sysadmin (with lfs ... command, lfs ladvise is too cryptic and I think it's best if you have something more intuitive thought up, things like lfs mark fastaccess file, to give a random nonbinding example), the implementation then would be to issue the same advise command (whatever would be the way to do this) and then issuing a 1 byte read or some other lighweight IO in the necessary region so that your wishes are transferred to the server.
Oh, also note how now if an application wants to control a file access, it needs to do two calls - to posix_Fadvise and your ioctl - this is also inconvenient, I imagine, and if the kernel guys reject your approach of calling into the filesystem - you might want to call into sys_fadvise64 yourself from your ioctl.
Oleg, I think you miss some of the vslue of this interface. The regular fadvise syscall is itself not necessarily storing any persistent state on the client either. fadvise(WILLNEED) just prefetches pages into cache, but has no guarantee they will be even loaded or kept, since it is only advice to try to optimize performance.
The ladvise code is similar to fadvise, except it is like calling fadvise on the server, which isn't possible today. Even if the upstream kernel was changed to allow fadvise() to contact the filesystem, the behavior is different. The workload for ladvise is e.g. a bunch of different clients are doing small random reads of a file, so prefetching pages into OSS cache with big linear reads before the random IO is a net benefit. Fetching all that data into each client cache with fadvise() may not be, due to much more data being sent to the client.
Similarly, having an ladvise DONTNEED that could flush all the cache for a specific object from OST cache may be better than only flushing it from the client cache.
Even if fadvise is changed in the upstream kernel, it will take several years before that got into widely used vendor kernels (I don't think we plan to oatch client kernels), so having an interface for current systems is needed.
Hi Oleg,
I think Andreas has pointed out all the possible reasons that I can think of. And please notice that another important usage of ladvise() interface is to give DSS hints to OSTs in the future. Ofcouse, we could hack fadvise() interface and add DSS advice. But DSS advice is so different with existing fadvise() framework that a seperate interface seems better.
Andreas: I got that. regular fadvise does store permanent state on the client in many cases even now (not in enough details to be useful here, though).
As for the local cache population on the client from fadvise - if we have a callback to fadvise into the filesystem - then we can control all of this to a big degree and I imagine this would be the first thing upstreeam guys ask us - i.e. "why did not you ask for a call into fs from sys_fadvise64"?
I wonder if other filesystems like GPFS have this sort of thing too?
I feel that your example of random reads from a file does not really address all problems at hand. In reality there are three possible cases: a single client does a lot of random reads from the file and a lot of clients do random reads from the file (causing or not causing entire file to be read in the end).
For the cases number 3 it's definitely beneficial to prefetch entire file into server cache and keep it there, for case number two (lots of clients, not entire file read) usefulness is a function of how big of a probability it is we'll hit adjacent blocks, as it could be
the reads also would be wasted if you have clients read random places around 10% of a totally huge file. And in 1st case with a single client - it makes no sense to prefetch anything on the server.
I imagine theanswer to the 1st case is not to do this call, but how to distinguish between case #2 and #3 not call anything for #3, have a different argument for #3 (i.e. for #3 we call with WILLNEED and for #2 and possible even #1 as RANDOM)?
Additionally (something not done in this patch, and not requiring protocol changes, but probably needed eventually) is to do client cache management accordingly. i.e. in all cases we'd need to shrink readahead windows to basically 0 and not allow them to grow. Or do we really plan to keep these completely separate - i.e. backend and frontend cache control? So people need to calls to control each one separately?
As for patching client servers, at least in case of RedHat they seem to be happy to backport patches from upstream that people need, as long as they were already accepted upstream. I imagine other major vendors are in a similar position.
Now, back to the protocol level changes - do we really think sending those advices separately, as opposed to a part of IO RPC is the right choice? Protocol changes are the most fixed in place and hardest to change so we really need to strike it right the first go around.
In my view embedding the advices into the IO RPCs has the benefit of actually making sure server has the actual picture of what's going on every time.
Since the advices are non-sticky (as per your comments) - I imagine then it's a bad idea to lose them (does not mean we need to grant them every time of course, server will have it's own logic about this) mid-run of the application just because server restarted?
On the downside, I imagine, if two different applications with different settings are working on the same file, a hilarity will ensue as they give conflicting commands with every RPC.
I guess if we do decide on the separate RPCs for advices, it makes sense to futureproof them a bit to allow multiple entries.
I think it may be best to consider client vs. server cache management separately, even if a functional fadvise() call was available in the future. If a client calls fadvise(DONTNEED), it isn't clear if that should also flush cache on the server (maybe other clients are still accessing that file).
If an app is sophisticated enough to use llapi_ladvise() then it can also call fadvise() as needed. Why should we entangle client and server cache management if there may be good reasons not to?
One example could be an app that uses ladvise(WILLNEED) to fetch random IO data into server cache via prefetch, reads randomly into client cache, then ladvise(DONTNEED) to drop it from the server cache to start loading the next dataset in advance while it is still in client cache until processing is done. The app doesn't want ladvise(DONTNEED) to flush the client cache, and if we entangle the two then such optimization wouldn't be possible.
Yes, it is possible that conflicting directives could be sent from multiple clients, but the OST could also start to ignore the advice in this case. In any case, in this case, the effect will generally be limited to a single user's files, so if they are asking for inconsistent behaviour then they get what they ask for in the end.
As for adding such directives with each IO RPC, I could imagine that might also be possible (e.g. server side prefetch for readahead), and that isn't precluded in the future, but I imagine that for most workloads the app will call ladvise() separately from read/write so it makes sense to have a separate RPC for it today. Also, I expect that ladvise() advice will only be needed when it is not something that the kernel could detect itself (e.g. that some random IO is coming soon) so it might never be possible to automatically generate such hints automatically from within the kernel.
Looking at the ongoing examples in the gerrit, it looks like we really have two usecases here.
One is for storage control where we tell the storage (and it's free to ignore of course) things like "this file is about to be pounded in a way that is best handled by storage tier XXX". I imagine this indeed is ok to send as separate RPCs.
The other is for ongoing IO like RANDOM designation that probably makes the most sense to send along with IO (a one time readahead window reset is probably less ideal).
Though if we step further away - they are the same. When we want to have random IO for an object - it makes sense to ask the storage to move it to some SSD-like tier with low latency, and then every IO would not need to be cached or read-ahead deeply because cache hit rate is going to be low.
As such I imagine the current patch is mostly fine as the very first step in that direction. Protocol-wise it's ok.
Do we really want the magic to be part of ioctl call? Is the plan here to use it a s a version check so different magic enables more features? But then we know extra features are enabled because we'll see them being used too, so that's kind of moot, and if we need to change the structure itself, then the ioctl number will change (it hashes the struct size into the actual ioctl number along with other stuff).
There are also some other style and correctness details in my first review that need to be addressed. And we also still probably should try and start the discussion in linux-fsdevel about feasibility of passing fadvise calls down to FS level so that when we get to usecases useful there - we already know the answer on possible avenues of implementing that?
One is for storage control where we tell the storage (and it's free to ignore of course) things like "this file is about to be pounded in a way that is best handled by storage tier XXX". I imagine this indeed is ok to send as separate RPCs.
I think this is exactly the problem this patch will address. I don't think random IO is in scope of this patch because dedicated API is provided to applications and the applications should know exactly what they are doing. It's simply for the applications to look for trouble to issue wrong IO model to OSTs.
I guess one confusion is from the function name fadivse(). This is why I'd like to avoid advise() similar stuff and using a totally different name.
The comments are too long, sorry if I missed something.
Actually I guess teh advise thing makes sense in a way, if we consider that it is application givilg the acces advice ahead of time.
It's just the RPCs proposed only make sense as advice to tiered storage on the backend to let it know ahead of time when to move stuff to different tiers ahead of time.
If the server cache can be considered a "storage tier" then this code is already useful. Also, for ZFS with L2ARC or DSS it also would be useful if wired in correctly.
Server cache is definitely a storage tier in my book.
And I agree the code is useful (once the server side support is added - that is).
I was thinking for LU_LADVISE_RANDOM that it makes sense to send the random IO blocksize with the ladvise RPC. For new file writes with ZFS this would allow selecting the blocksize of the file to match the random IO size to avoid large read-modify-writes.
Yeah, that makes sense. And in the process of adding advice type support for DSS, I also realized that extra fields of 'struct lu_ladvise' might be necessary for specifying arguments. That requires wire protocol updates. I am not sure how these arguments could be added in a extendable way, because one or two u64 padding fields does not sound like a good solution.
I've cleaned up the codes again. A manual of lfs-ladvise has been added. WILLNEED advice has been renamed to WILLREAD to prevent confusion with Linux kernel fadvise.
Would it be possible (or reasonable) to add a lockless_truncate flag into the ladvise framework? We have customers who would like more fine-grained control over the lockless_truncate flag (by file, rather than by filesystem).
Ben,
That is really a client-side optimization instead of a server-side optimization, but I guess it could also be added into the "hint for Lustre file IO access" area. There is already an ioctl for lockless IO, but that is overkill for what you want.
You could propose a patch and we can see what it looks like. It would just be a new advice type, and set a flag on the Lustre file info that truncates are lockless, unless the client already has a lock.
Add ticket for manual update.
If the purpose of the advice is to influence backend tier selection, it also probably makes sense to include a ADVICE_ARCHIVE directive indicating that data should be down-tiered to HSM or slower Lustre storage.
Is that a different interface for "lfs hsm_archive" on the file, or are you thinking if HSM archives behind individual OSTs? I'm not against adding advices, but I think they need to have a real use case and not just something that might be used in the future.
Most flags listed here are for cache / hot data; I'm suggesting it is helpful to know data non-re-use as well: ADVICE_CACHE_CLIENT -> ADVICE_CACHE_SERVER -> ADVICE_UNCACHE -> ADVICE_ARCHIVE. For example, if I'm writing a checkpoint file that I know I will not read, Lustre might choose to follow the DIO path and skip all caches. It was just a thought for consideration really; I'm not trying to push it.
Oleg Drokin (oleg.drokin@intel.com) merged in patch http://review.whamcloud.com/10029/
Subject: LU-4931 ladvise: Add feature of giving file access advices
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: e14246641c04c9e3004043f58f469532223d06d6
Li Xi (lixi@ddn.com) uploaded a new patch: http://review.whamcloud.com/20203
Subject: LU-4931 ladvise: Add noread advice support for ladvise
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 4404ced63b358383825199a5904d0c2b772fe9b0
Oleg Drokin (oleg.drokin@intel.com) merged in patch http://review.whamcloud.com/12458/
Subject: LU-4931 ladvise: Add willread advice support for ladvise
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: f756979d9730333394037f127e75f43910174622
Gu Zheng (gzheng@ddn.com) uploaded a new patch: http://review.whamcloud.com/21940
Subject: LU-4931 ladvise: add code for ladvise_hdr into wirecheck.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 1b888505ff79dd4fcb42197df972c96033b57f19
James Nunez (james.a.nunez@intel.com) uploaded a new patch: http://review.whamcloud.com/22361
Subject: LU-4931 tests: Run ladvise DONTNEED Test Multiple Times
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 4713ee435075ff3818a593572028bb8058267c55
Li Xi, I noticed just now in ofd_ladvise_prefetch() that this is allocating PTLRPC_MAX_BRW_PAGES * sizeof(niobuf_local) = 160KB for each ladvise willread call. Instead, this should be using struct tgt_thread_big_cache *tbc = req->rq_svc_thread->t_data as is done in tgt_brw_read().
Also, while you are in there, can you please fix the indenting for
static int ofd_ladvise_hdl(struct tgt_session_info *tsi) { : : case LU_LADVISE_WILLREAD: req->rq_status = ofd_ladvise_prefetch(env, fo, ladvise->lla_start, ladvise->lla_end);
to be
req->rq_status = ofd_ladvise_prefetch(env, fo,
ladvise->lla_start,
ladvise->lla_end);
Li Xi (lixi@ddn.com) uploaded a new patch: http://review.whamcloud.com/22489
Subject: LU-4931 ofd: use thread buffer for ladvise
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 64b359ee1aa58b95c13323039b300a04556ff033
Oleg Drokin (oleg.drokin@intel.com) merged in patch http://review.whamcloud.com/20203/
Subject: LU-4931 ladvise: Add dontneed advice support for ladvise
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: a5a7890093ea2509db15f8aa8a8c9d9c86133209
As per discussion with Ihara all further enhancements in this area will be tracked under a different JIRA ticket
Reopen to land man page for 2.9.0:
Andreas Dilger (andreas.dilger@intel.com) uploaded a new patch: http://review.whamcloud.com/22910
Subject: LU-4931 doc: update ladvise man page
Project: fs/lustre-release
Branch: master
Current Patch Set: 2
Commit: 61ceb43c858d6fc979fc4da9d2a925026b27859a
Oleg Drokin (oleg.drokin@intel.com) merged in patch http://review.whamcloud.com/22910/
Subject: LU-4931 doc: update ladvise man page
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: f63e53a364f5162c0f8a81e42978c5a2b9b7522d
Man page has landed. Remaining patches tracked under this id will be landed under a new ticket
Oleg Drokin (oleg.drokin@intel.com) merged in patch http://review.whamcloud.com/22489/
Subject: LU-4931 ofd: use thread buffer for ladvise
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: c29cf72acd431e65f0438804561e7c30feef0366
Oleg Drokin (oleg.drokin@intel.com) merged in patch http://review.whamcloud.com/21940/
Subject: LU-4931 ladvise: add code for ladvise_hdr into wirecheck.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 5ee1287305fb6b6c472d097ef9a86a9e315104e4