lkml.org 
[lkml]   [2019]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH RESEND 1/1] vfs: Really check for inode ptr in lookup_fast
On Sat, Nov 02, 2019 at 06:17:06AM +0000, Al Viro wrote:
> On Fri, Nov 01, 2019 at 11:46:22PM +0000, Al Viro wrote:
> > on anything except alpha that would be pretty much automatic and
> > on alpha we get the things along the lines of
> >
> > f = fdt[n]
> > mb
> > d = f->f_path.dentry
> > i = d->d_inode
> > assert(i != NULL)
> > vs.
> > see that d->d_inode is non-NULL
> > f->f_path.dentry = d
> > mb
> > fdt[n] = f
> >
> > IOW, the barriers that make it safe to fetch the fields of struct file
> > (rcu_dereference_raw() in __fcheck_files() vs. smp_store_release()
> > in __fd_install() in the above) should *hopefully* take care of all
> > stores visible by the time of do_dentry_open(). Sure, alpha cache
> > coherency is insane, but AFAICS it's not _that_ insane.
> >
> > Question to folks familiar with alpha memory model:
> >
> > A = 0, B = NULL, C = NULL
> > CPU1:
> > A = 1
> >
> > CPU2:
> > r1 = A
> > if (r1) {
> > B = &A
> > mb
> > C = &B
> > }
> >
> > CPU3:
> > r2 = C;
> > mb
> > if (r2) { // &B
> > r3 = *r2 // &A
> > r4 = *r3 // 1
> > assert(r4 == 1)
> > }
> >
> > is the above safe on alpha?
>
> Hmm... After digging through alpha manuals, it should be -
>
> U1: W A, 1
>
> V1: R A, 1

Assuming a compare and branch here ...

> V2: W B, &A
> V3: MB
> V4: W C, &B
>
> W1: R C, &B
> W2: MB

... and here ...

> W3: R B, &A
> W4: R A, 0
>
> is rejected since
> U1 BEFORE V1 (storage and visibility)
> V1 BEFORE V3 BEFORE V4 (processor issue order constraints)
> V4 BEFORE W1 (storage and visibility)
> W1 BEFORE W2 BEFORE W4 (processor issue order constraints)
> and W4 BEFORE U1 (storage and visibility), which is impossible
> due to BEFORE being acyclic and transitive.
>
> I might very well be missing something, though... Paul, could you
> take a look and tell if the above makes sense?

... then yes, agreed. Alpha does respect control dependencies to
stores, and you supplied the required mb for the last task that has
a control dependency only to loads.

I have to ask... Are you seeing failures on Alpha?

Thanx, Paul

\
 
 \ /
  Last update: 2019-11-02 18:25    [W:0.210 / U:0.120 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site