On Thu, Apr 28, 2022 at 04:51:49PM +0530, Sai Prakash Ranjan wrote:
> On 4/28/2022 4:21 PM, Greg KH wrote:
> > On Thu, Apr 28, 2022 at 03:25:30PM +0530, Sai Prakash Ranjan wrote:
> > > Add logging support for MMIO high level accessors such as read{b,w,l,q}
> > > and their relaxed versions to aid in debugging unexpected crashes/hangs
> > > caused by the corresponding MMIO operation.
> > >
> > > Signed-off-by: Sai Prakash Ranjan <[email protected]>
> > > ---
> > > include/asm-generic/io.h | 82 ++++++++++++++++++++++++++++++++++++++--
> > > 1 file changed, 78 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > > index 7ce93aaf69f8..99090722cb4b 100644
> > > --- a/include/asm-generic/io.h
> > > +++ b/include/asm-generic/io.h
> > > @@ -10,6 +10,7 @@
> > > #include <asm/page.h> /* I/O is all done through memory accesses */
> > > #include <linux/string.h> /* for memset() and memcpy() */
> > > #include <linux/types.h>
> > > +#include <linux/instruction_pointer.h>
> > > #ifdef CONFIG_GENERIC_IOMAP
> > > #include <asm-generic/iomap.h>
> > > @@ -61,6 +62,35 @@
> > > #define __io_par(v) __io_ar(v)
> > > #endif
> > > +#if IS_ENABLED(CONFIG_TRACE_MMIO_ACCESS) && !(defined(__DISABLE_TRACE_MMIO__))
> > Shouldn't you document __DISABLE_TRACE_MMIO__ somewhere? It's not even
> > in the changelog. Put a big comment above this for what is is for and
> > how to use it. Otherwise you will forget all about this in 6 months :)
> >
> > thanks,
> >
> > greg k-h
>
> Didn't you ask me to split the patch to the one actually adding the flag and the one using it.
Yes, and isn't this the commit that adds the flag? Or was that on an
earlier one that I missed?
Ah, it's in patch 6/9
But you should also document it here in the .h file, otherwise the only
place it is described is in some random kvm Makefile that no one will
ever notice :)
thanks,
greg k-h
On 4/28/2022 5:32 PM, Greg KH wrote:
> On Thu, Apr 28, 2022 at 04:51:49PM +0530, Sai Prakash Ranjan wrote:
>> On 4/28/2022 4:21 PM, Greg KH wrote:
>>> On Thu, Apr 28, 2022 at 03:25:30PM +0530, Sai Prakash Ranjan wrote:
>>>> Add logging support for MMIO high level accessors such as read{b,w,l,q}
>>>> and their relaxed versions to aid in debugging unexpected crashes/hangs
>>>> caused by the corresponding MMIO operation.
>>>>
>>>> Signed-off-by: Sai Prakash Ranjan <[email protected]>
>>>> ---
>>>> include/asm-generic/io.h | 82 ++++++++++++++++++++++++++++++++++++++--
>>>> 1 file changed, 78 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>>>> index 7ce93aaf69f8..99090722cb4b 100644
>>>> --- a/include/asm-generic/io.h
>>>> +++ b/include/asm-generic/io.h
>>>> @@ -10,6 +10,7 @@
>>>> #include <asm/page.h> /* I/O is all done through memory accesses */
>>>> #include <linux/string.h> /* for memset() and memcpy() */
>>>> #include <linux/types.h>
>>>> +#include <linux/instruction_pointer.h>
>>>> #ifdef CONFIG_GENERIC_IOMAP
>>>> #include <asm-generic/iomap.h>
>>>> @@ -61,6 +62,35 @@
>>>> #define __io_par(v) __io_ar(v)
>>>> #endif
>>>> +#if IS_ENABLED(CONFIG_TRACE_MMIO_ACCESS) && !(defined(__DISABLE_TRACE_MMIO__))
>>> Shouldn't you document __DISABLE_TRACE_MMIO__ somewhere? It's not even
>>> in the changelog. Put a big comment above this for what is is for and
>>> how to use it. Otherwise you will forget all about this in 6 months :)
>>>
>>> thanks,
>>>
>>> greg k-h
>> Didn't you ask me to split the patch to the one actually adding the flag and the one using it.
> Yes, and isn't this the commit that adds the flag? Or was that on an
> earlier one that I missed?
>
> Ah, it's in patch 6/9
>
> But you should also document it here in the .h file, otherwise the only
> place it is described is in some random kvm Makefile that no one will
> ever notice :)
>
>
Sure it makes sense, will add it.
Thanks,
Sai