Re: [PATCH] lightnvm: pblk: Add read memory barrier when reading from rb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/28/2018 01:31 AM, Heiner Litz wrote:
There is a control dependency between two disjoint variables (only read data if flags == WRITTEN). Because x86-TSO allows re-ordering of loads the control dependency can be violated.

I'm sorry, I do not see it :)

Here is my understanding:

entry->w_ctx.flags is used as a flagging mechanism to wait for data to become available when "flags" has PBLK_WRITTEN_DATA.

The later access to entry->data is independent of this. It is assumed that entry->w_ctx.flags is the guarding barrier.

Here is the titbit from the control dependency section that makes me say that the entry->data loads is not possible to be done before entry->w_ctx.flags:

" (*) Control dependencies apply only to the then-clause and else-clause
      of the if-statement containing the control dependency, including
      any functions that these two clauses call.  Control dependencies
      do -not- apply to code following the if-statement containing the
      control dependency."

The read of entry->data must come after the READ_ONCE of entry->w_ctx.flags.


https://www.kernel.org/doc/Documentation/memory-barriers.txt refers to the above scenario as control dependency but your description is of course also correct. Let me know if we should clarify the comment.

BTW: ARM has an even more relaxed memory model and also allows to re-order writes. If we want to support ARM correctly, I think we also need to insert a memory barrier between writing data and writing flags (and there might be a couple other places where we need to check).

On Fri, Jun 22, 2018, 11:02 AM Matias Bjørling <mb@xxxxxxxxxxx <mailto:mb@xxxxxxxxxxx>> wrote:

    On 06/21/2018 12:54 AM, Heiner Litz wrote:
     > READ_ONCE does not imply a read memory barrier in the presence of
    control
     > dependencies between two separate memory locations (flags and
    data). On x86
     > TSO, reading from the data page might be reordered before the
    flags read.
     > See chapter CONTROL DEPENDENCIES in
     > https://www.kernel.org/doc/Documentation/memory-barriers.txt
     >
     > Signed-off-by: Heiner Litz <hlitz@xxxxxxxx <mailto:hlitz@xxxxxxxx>>
     > ---
     >   drivers/lightnvm/pblk-rb.c | 3 +++
     >   1 file changed, 3 insertions(+)
     >
     > diff --git a/drivers/lightnvm/pblk-rb.c b/drivers/lightnvm/pblk-rb.c
     > index a81a97e..5f09983 100644
     > --- a/drivers/lightnvm/pblk-rb.c
     > +++ b/drivers/lightnvm/pblk-rb.c
     > @@ -545,6 +545,9 @@ unsigned int pblk_rb_read_to_bio(struct
    pblk_rb *rb, struct nvm_rq *rqd,
     >                       goto try;
     >               }
     >
     > +             /* Observe control dependency between flags and
    data read */
     > +             smp_rmb();
     > +
     >               page = virt_to_page(entry->data);
     >               if (!page) {
     >                       pr_err("pblk: could not allocate write bio
    page\n");
     >

    Hi Heiner,

    Can you help explain how it is a control dependency? What case am I
    missing?

    The way I read the code, there is the case where a read of entry->data
    happens before entry->w_ctx.flags, but I see that as a memory
    reordering, which the smp_rmb() fixes. Would that be more accurate?

    Thank you,
    Matias





[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux