On Thursday 09 January 2020 13:56:57 Pali Rohár wrote:
> On Thursday 09 January 2020 13:44:05 Jan Kara wrote:
> > On Wed 08-01-20 23:32:40, Pali Rohár wrote:
> > > On Wednesday 08 January 2020 13:19:19 Jan Kara wrote:
> > > > Free space on filesystems with metadata or virtual partition maps
> > > > currently gets misreported. This is because these partitions are just
> > > > remapped onto underlying real partitions from which keep track of free
> > > > blocks. Take this remapping into account when counting free blocks as
> > > > well.
> > > >
> > > > Reported-by: Pali Rohár <pali.rohar@xxxxxxxxx>
> > > > Signed-off-by: Jan Kara <jack@xxxxxxx>
> > > > ---
> > > > fs/udf/super.c | 19 ++++++++++++++-----
> > > > 1 file changed, 14 insertions(+), 5 deletions(-)
> > > >
> > > > I plan to take this patch to my tree.
> > > >
> > > > diff --git a/fs/udf/super.c b/fs/udf/super.c
> > > > index 8c28e93e9b73..b89e420a4b85 100644
> > > > --- a/fs/udf/super.c
> > > > +++ b/fs/udf/super.c
> > > > @@ -2492,17 +2492,26 @@ static unsigned int udf_count_free_table(struct super_block *sb,
> > > > static unsigned int udf_count_free(struct super_block *sb)
> > > > {
> > > > unsigned int accum = 0;
> > > > - struct udf_sb_info *sbi;
> > > > + struct udf_sb_info *sbi = UDF_SB(sb);
> > > > struct udf_part_map *map;
> > > > + unsigned int part = sbi->s_partition;
> > > > + int ptype = sbi->s_partmaps[part].s_partition_type;
> > > > +
> > > > + if (ptype == UDF_METADATA_MAP25) {
> > > > + part = sbi->s_partmaps[part].s_type_specific.s_metadata.
> > > > + s_phys_partition_ref;
> > > > + } else if (ptype == UDF_VIRTUAL_MAP15 || ptype == UDF_VIRTUAL_MAP20) {
> > > > + part = UDF_I(sbi->s_vat_inode)->i_location.
> > > > + partitionReferenceNum;
> > >
> > > Hello! I do not think that it make sense to report "free blocks" for
> > > discs with Virtual partition. By definition of VAT, all blocks prior to
> > > VAT are already "read-only" and therefore these blocks cannot be use for
> > > writing new data by any implementation. And because VAT is stored on the
> > > last block, in our model all blocks are "occupied".
> >
> > Fair enough. Let's just always return 0 for disks with VAT partition.
> >
> > > > + }
> > > >
> > > > - sbi = UDF_SB(sb);
> > > > if (sbi->s_lvid_bh) {
> > > > struct logicalVolIntegrityDesc *lvid =
> > > > (struct logicalVolIntegrityDesc *)
> > > > sbi->s_lvid_bh->b_data;
> > > > - if (le32_to_cpu(lvid->numOfPartitions) > sbi->s_partition) {
> > > > + if (le32_to_cpu(lvid->numOfPartitions) > part) {
> > > > accum = le32_to_cpu(
> > > > - lvid->freeSpaceTable[sbi->s_partition]);
> > > > + lvid->freeSpaceTable[part]);
> > >
> > > And in any case freeSpaceTable should not be used for discs with VAT.
> > > And we should ignore its value for discs with VAT.
> > >
> > > UDF 2.60 2.2.6.2: Free Space Table values be maintained ... except ...
> > > for a virtual partition ...
> > >
> > > And same applies for "partition with Access Type pseudo-overwritable".
> >
> > Well this is handled by the 'accum == 0xffffffff' condition below. So we
> > effectively ignore these values.
>
> Ok.
Now I'm thinking about another scenario: UDF allows you to have two
partitions of Type1 (physical) on one volume: one with read-only access
type and one with overwritable access type.
UDF 2.60 2.2.6.2 says: For a partition with Access Type read-only, the
Free Space Table value shall be set to zero. And therefore we should
ignore it.
But current implementation for discs without Metadata partition (all
with UDF 2.01) reads free space table (only) from partition
unsigned int part = sbi->s_partition;
So is this s_partition one with read-only or overwritable access type?
And to make it more complicated, UDF 2.60 2.2.10 requires that such discs
(with two partitions) needs to have also Metadata Partition Map.
> > > > if (accum == 0xFFFFFFFF)
> > > > accum = 0;
> > > > }
> >
> > Honza
>
--
Pali Rohár
pali.rohar@xxxxxxxxx
