On Sun, May 26, 2019 at 12:09 AM Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
<snip>
> Forward-declaring inline functions is peculiar, but it does appear to work.
>
> z3fold is quite inline-happy. Fortunately the compiler will ignore the
> inline hint if it seems a bad idea. Even then, the below shrinks
> z3fold.o text from 30k to 27k. Which might even make it faster....
It is faster with inlines, I'll try to find a better balance between
size and performance in the next version of the patch though.
<snip>
> >
> > ...
> >
> > +static inline struct z3fold_header *__get_z3fold_header(unsigned long handle,
> > + bool lock)
> > +{
> > + struct z3fold_buddy_slots *slots;
> > + struct z3fold_header *zhdr;
> > + unsigned int seq;
> > + bool is_valid;
> > +
> > + if (!(handle & (1 << PAGE_HEADLESS))) {
> > + slots = handle_to_slots(handle);
> > + do {
> > + unsigned long addr;
> > +
> > + seq = read_seqbegin(&slots->seqlock);
> > + addr = *(unsigned long *)handle;
> > + zhdr = (struct z3fold_header *)(addr & PAGE_MASK);
> > + preempt_disable();
>
> Why is this done?
>
> > + is_valid = !read_seqretry(&slots->seqlock, seq);
> > + if (!is_valid) {
> > + preempt_enable();
> > + continue;
> > + }
> > + /*
> > + * if we are here, zhdr is a pointer to a valid z3fold
> > + * header. Lock it! And then re-check if someone has
> > + * changed which z3fold page this handle points to
> > + */
> > + if (lock)
> > + z3fold_page_lock(zhdr);
> > + preempt_enable();
> > + /*
> > + * we use is_valid as a "cached" value: if it's false,
> > + * no other checks needed, have to go one more round
> > + */
> > + } while (!is_valid || (read_seqretry(&slots->seqlock, seq) &&
> > + (lock ? ({ z3fold_page_unlock(zhdr); 1; }) : 1)));
> > + } else {
> > + zhdr = (struct z3fold_header *)(handle & PAGE_MASK);
> > + }
> > +
> > + return zhdr;
> > +}
> >
> > ...
> >
> > static unsigned short handle_to_chunks(unsigned long handle)
> > {
> > - unsigned long addr = *(unsigned long *)handle;
> > + unsigned long addr;
> > + struct z3fold_buddy_slots *slots = handle_to_slots(handle);
> > + unsigned int seq;
> > +
> > + do {
> > + seq = read_seqbegin(&slots->seqlock);
> > + addr = *(unsigned long *)handle;
> > + } while (read_seqretry(&slots->seqlock, seq));
>
> It isn't done here (I think).
handle_to_chunks() is always called with z3fold header locked which
makes it a lot easier in this case. I'll add some comments in V2.
Thanks,
Vitaly
