Re: [PATCH v3 1/2] iommu/arm-smmu-v3: Print better events records

[Pranjal Shrivastava <praan@google.com>]

On Wed, Oct 02, 2024 at 10:22:13AM -0700, Nicolin Chen wrote:
> On Wed, Oct 02, 2024 at 02:10:52PM -0300, Jason Gunthorpe wrote:
> > On Wed, Oct 02, 2024 at 09:55:14AM -0700, Nicolin Chen wrote:
> > > > dev_name()'s result can't leave the lock either, at least not with out
> > > > some refcounting on the struct device.

Agreed, I missed that! Maybe we can avoid reading the master name in the
`arm_smmu_get_evt_info` and move it within the `arm_smmu_handle_evt`
under proper locking? Since the event won't be dumped before it's
handled, we can avoid locking at two places for doing the same thing?

> > >
> > > I found dev_name returns "const char *" so the string source is
> > > likely in the data section? If so, could the result of a locked
> > > "master_name = dev_name()" get away from the lock?
> > 
> > The name is allocated memory freed when the struct device is freed.
> 
> Oh, reading it a bit further, I found kfree_const() at kobj->name
> and some of the dev->init_names aren't coming from static strings.
> 
> We could kstrdup alternatively, but keeping the dump() inside the
> lock is just simpler.

kstrdup is a good idea too.. overall, I wouldn't wanna *accidentally*
mess up locking for logging (no rhymes intended).

Hence, we have 2 options:

1. Read the master_name within `arm_smmu_handle_evt` (at one place only)
2. Use kstrdup to copy the master_name elsewhere and dereference it.

Personally, I won't prefer 2 as we'd also have to free the memory
allocated for the dup'ed string after logging everything in
`arm_smmu_handle_evt` which is a bottom-half. LMK what y'all think?

> 
> Thanks!
> Nicolin

Thanks,
Pranjal