linux-kernel - Re: [RFC PATCH] mm: trace filemap add and del

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 21 Nov 2012 11:36:09 +0100
From:	Robert Jarzmik <robert.jarzmik@...e.fr>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] mm: trace filemap add and del

Andrew Morton <akpm@...ux-foundation.org> writes:
>> +	TP_STRUCT__entry(
>> +		__field(struct page *, page)
>> +		__field(unsigned long, i_no)
>
> May as well call this i_ino - there's little benefit in using a
> different identifier.
Agreed for patch V2.
>
>> +		__field(unsigned long, pageofs)
>
> "index".
Agreed for patch V2.

>
>> +		__field(dev_t, s_dev)
>
> Perhaps use super_block.s_id here
If you imply by that that dereferencing page->mapping->host->i_sb and looking
for field s_dev, then I don't agree. Sometimes i_sb is NULL from what I recall,
especially in cases where the read page is from a journaling partition.

So unless I didn't understand you, I'll keep this part as well as the following,
to cover :
 - a mapping of an actual file
 - a mapping of a partition block

>
>> +	),
>> +
>> +	TP_fast_assign(
>> +		__entry->page = page;
>> +		__entry->i_no = page->mapping->host->i_ino;
>> +		__entry->pageofs = page->index;
>> +		if (page->mapping->host->i_sb)
>> +			__entry->s_dev = page->mapping->host->i_sb->s_dev;
>> +		else
>> +			__entry->s_dev = page->mapping->host->i_rdev;
>
> and hence avoid all this stuff.
See above.

>
>> +	),
>> +
>> +	TP_printk("page=%p pfn=%lu blk=%d:%d inode+ofs=%lu+%lu",
>> +		__entry->page,
>> +		page_to_pfn(__entry->page),
>> +		MAJOR(__entry->s_dev), MINOR(__entry->s_dev),
>> +		__entry->i_no,
>> +		__entry->pageofs << PAGE_SHIFT)
>> +);
>> +
>> +TRACE_EVENT(mm_filemap_add_to_page_cache,
>
Agreed for patch V2.

Thanks for the review.

-- 
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/