On Sat, Oct 01, 2016 at 10:54:36AM -0600, Jason Gunthorpe wrote:
> On Sat, Oct 01, 2016 at 03:01:25PM +0300, Jarkko Sakkinen wrote:
>
> > > + struct tpm_securityfs_data bin_sfs_data;
> > > + struct tpm_securityfs_data ascii_sfs_data;
> >
> > I think this is otherwise right but the struct name is very clunky.
> > First of all it doesn't own the data and IMHO now it kind of implies
> > of owning.
Ok, I'm not going to make this an issue. If you think these are good
names, I'll live with that :)
> These are passed in here:
>
> > > chip->bios_dir[chip->bios_dir_count] =
> > > securityfs_create_file("ascii_bios_measurements",
> > > S_IRUSR | S_IRGRP, chip->bios_dir[0],
> > > - (void *)&tpm_ascii_b_measurments_seqops,
> > > + (void *)&chip->ascii_sfs_data,
> > > &tpm_bios_measurements_ops);
>
> And the argument to securityfs_create_file is called 'data'..
>
> The key question with these patches is if all the locking is done
> right and we have the correct lifetime model now.
>
> Eg how much does securityfs_remove serialize and is the kref on the
> chip held for the duration of any fops. Can open() start after the
> kref is dropped, etc.
Why not make tpm_securityfs_data refcounted in order to remove
binding to the chip?
/Jarkko
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
