On Fri, Aug 25, 2017 at 3:19 PM, Tim Chen <tim.c.chen@linux.intel.com> wrote:
>
> Also I think patch 1 is still a good idea for a fail safe mechanism
> in case there are other long wait list.

Yeah, I don't hate patch #1.

But that other patch is just nasty.

> That said, I do think your suggested approach is cleaner.  However, it
> is a much more substantial change.  Let me take a look and see if I
> have any issues implementing it.

Actually, I tried it myself.

It was painful. But I actually have a TOTALLY UNTESTED set of two
patches that implements the idea.

And by "implements the idea" I mean "it kind of compiles, and it kind
of looks like it might work".

But by "kind of compiles" I mean that I didn't implement the nasty
add_page_wait_queue() thing that the cachefiles interface wants.
Honestly, I think the sanest way to do that is to just have a hashed
wait queue *just* for cachefiles.

And by "kind of looks like it might work" I really mean just that.
It's entirely untested. It's more of a "let's take that description of
mine and turn it into code". I really have not tested this AT ALL.

And it's subtle enough that I suspect it really is majorly buggy. It
uses the locking hash list code (hlist_bl_node) to keep the hash table
fairly small and hide the lock in the hash table itself.

And then it plays horrible games with linked lists. Yeah, that may
have been a mistake, but I thought I should try to avoid the doubly
linked lists in that "struct page_wait_struct" because it's allocated
on the stack, and each list_head is 16 bytes on 64-bit architectures.

But that "let's save 24 bytes in the structure" made it much nastier
to remove entries, so it was probably a bad trade-off.

But I'm attaching the two patches because I have no shame. If somebody
is willing to look at my completely untested crap code.

I *really* want to emphasize that "untested crap".

This is meant to be example code of the *concept* rather than anything
that actually works.

So take it as that: example pseudo-code that happens to pass a
compiler, but is meant as a RFD rather than actually working.

The first patch just moves code around because I wanted to experiment
with the new code in a new file. That first patch is probably fine. It
shouldn't change any code, just move it.

The second patch is the "broken patch to illustrate the idea".

                    Linus