Re: [PATCH v4 12/17] net: cxgb4/cxgb4vf: Eliminate duplicate barriers on weakly-ordered archs

[okaya]

On 2018-03-21 19:03, Casey Leedom wrote:
> [[ Appologies for the DUPLICATE email.  I forgot to tell my Mail Agent 
> to
>    use Plain Text. -- Casey ]]
>
>   I feel very uncomfortable with these proposed changes.  Our team is 
> right
> in the middle of trying to tease our way through the various platform
> implementations of writel(), writel_relaxed(), __raw_writel(), etc. in 
> order
> to support x86, PowerPC, ARM, etc. with a single code base.  This is
> complicated by the somewhat ... "fuzzily defined" semantics and varying
> platform implementations of all of these APIs.  (And note that I'm just
> picking writel() as an example.)
>
>   Additionally, many of the changes aren't even in fast paths and are 
> thus
> unneeded for performance.
>
>   Please don't make these changes.  We're trying to get this all sussed 
> out.

I was also given the feedback to look at performance critical path only. 
I am in the process of revisiting the patches.

If you can point me to the ones that are important, I can try to limit 
the changes to those only.

If your team wants to do it, I can drop this patch as well.

I think the semantics of write API is clear. What was actually 
implemented is another story.

I can share a few of my findings.

A portable driver needs to do this.

descriptor update in mem
wmb ()
writel_relaxed ()
mmiowb ()

Using __raw_write() is wrong as it can get reordered.

Using wmb()+writel() is also wrong for performance reasons.

If something is unclear, please ask.

>