Joe Perches writes:
> On Tue, 2017-12-12 at 08:43 +1100, Dave Chinner wrote:
>> On Sat, Dec 09, 2017 at 09:00:18AM -0800, Joe Perches wrote:
>> > On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote:
>> > > 1. Using lockdep_set_novalidate_class() for anything other
>> > >
On Mon, 2017-12-11 at 14:43 -0800, Matthew Wilcox wrote:
> - There's no warning for the first paragraph of section 6:
>
> 6) Functions
>
>
> Functions should be short and sweet, and do just one thing. They should
> fit on one or two screenfuls of text (the ISO/ANSI screen size is
On Mon, 11 Dec 2017, Joe Perches wrote:
> > - I don't understand the error for xa_head here:
> >
> > struct xarray {
> > spinlock_t xa_lock;
> > gfp_t xa_flags;
> > void __rcu *xa_head;
> > };
> >
> >Do people really think that:
> >
> > struct
On Mon, Dec 11, 2017 at 02:12:28PM -0800, Joe Perches wrote:
> On Tue, 2017-12-12 at 08:43 +1100, Dave Chinner wrote:
> > On Sat, Dec 09, 2017 at 09:00:18AM -0800, Joe Perches wrote:
> > > On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote:
> > > > 1. Using
On Mon, 2017-12-11 at 14:43 -0800, Matthew Wilcox wrote:
> On Mon, Dec 11, 2017 at 02:12:28PM -0800, Joe Perches wrote:
> > Completely reasonable. Thanks.
>
> If we're doing "completely reasonable" complaints, then ...
>
> - I don't understand why plain 'unsigned' is deemed bad.
That was a
On Mon, Dec 11, 2017 at 02:12:28PM -0800, Joe Perches wrote:
> Completely reasonable. Thanks.
If we're doing "completely reasonable" complaints, then ...
- I don't understand why plain 'unsigned' is deemed bad.
- The rule about all function parameters in prototypes having a name
doesn't
On Tue, 2017-12-12 at 08:43 +1100, Dave Chinner wrote:
> On Sat, Dec 09, 2017 at 09:00:18AM -0800, Joe Perches wrote:
> > On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote:
> > > 1. Using lockdep_set_novalidate_class() for anything other
> > > than device->mutex will throw checkpatch
On Sun, Dec 10, 2017 at 08:23:15PM -0800, Matthew Wilcox wrote:
> On Mon, Dec 11, 2017 at 10:57:45AM +1100, Dave Chinner wrote:
> > i.e. the fact the cmpxchg failed may not have anything to do with a
> > race condtion - it failed because the slot wasn't empty like we
> > expected it to be. There
On Sat, Dec 09, 2017 at 09:00:18AM -0800, Joe Perches wrote:
> On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote:
> > 1. Using lockdep_set_novalidate_class() for anything other
> > than device->mutex will throw checkpatch warnings. Nice. (*)
> []
> > (*) checkpatch.pl is considered
On Mon, Dec 11, 2017 at 10:57:45AM +1100, Dave Chinner wrote:
> i.e. the fact the cmpxchg failed may not have anything to do with a
> race condtion - it failed because the slot wasn't empty like we
> expected it to be. There can be any number of reasons the slot isn't
> empty - the API should not
On Fri, Dec 08, 2017 at 03:01:31PM -0800, Matthew Wilcox wrote:
> On Thu, Dec 07, 2017 at 11:38:43AM +1100, Dave Chinner wrote:
> > > > cmpxchg is for replacing a known object in a store - it's not really
> > > > intended for doing initial inserts after a lookup tells us there is
> > > > nothing
On Sat, 2017-12-09 at 09:36 +1100, Dave Chinner wrote:
> 1. Using lockdep_set_novalidate_class() for anything other
> than device->mutex will throw checkpatch warnings. Nice. (*)
[]
> (*) checkpatch.pl is considered mostly harmful round here, too,
> but that's another rant
How so?
On Thu, Dec 07, 2017 at 11:38:43AM +1100, Dave Chinner wrote:
> > > cmpxchg is for replacing a known object in a store - it's not really
> > > intended for doing initial inserts after a lookup tells us there is
> > > nothing in the store. The radix tree "insert only if empty" makes
> > > sense
On Fri, Dec 08, 2017 at 12:35:07PM -0500, Alan Stern wrote:
> On Fri, 8 Dec 2017, Byungchul Park wrote:
>
> > I'm sorry to hear that.. If I were you, I would also get
> > annoyed. And.. thanks for explanation.
> >
> > But, I think assigning lock classes properly and checking
> > relationship of
On Fri, 8 Dec 2017, Byungchul Park wrote:
> I'm sorry to hear that.. If I were you, I would also get
> annoyed. And.. thanks for explanation.
>
> But, I think assigning lock classes properly and checking
> relationship of the classes to detect deadlocks is reasonable.
>
> In my opinion about
On 12/8/2017 4:25 PM, Dave Chinner wrote:
On Fri, Dec 08, 2017 at 01:45:52PM +0900, Byungchul Park wrote:
On Fri, Dec 08, 2017 at 09:22:16AM +1100, Dave Chinner wrote:
On Thu, Dec 07, 2017 at 11:06:34AM -0500, Theodore Ts'o wrote:
On Wed, Dec 06, 2017 at 06:06:48AM -0800, Matthew Wilcox
On Fri, Dec 08, 2017 at 01:45:52PM +0900, Byungchul Park wrote:
> On Fri, Dec 08, 2017 at 09:22:16AM +1100, Dave Chinner wrote:
> > On Thu, Dec 07, 2017 at 11:06:34AM -0500, Theodore Ts'o wrote:
> > > On Wed, Dec 06, 2017 at 06:06:48AM -0800, Matthew Wilcox wrote:
> > > > > Unfortunately for you,
On Fri, Dec 08, 2017 at 09:22:16AM +1100, Dave Chinner wrote:
> On Thu, Dec 07, 2017 at 11:06:34AM -0500, Theodore Ts'o wrote:
> > On Wed, Dec 06, 2017 at 06:06:48AM -0800, Matthew Wilcox wrote:
> > > > Unfortunately for you, I don't find arguments along the lines of
> > > > "lockdep will save us"
On Thu, Dec 07, 2017 at 11:06:34AM -0500, Theodore Ts'o wrote:
> On Wed, Dec 06, 2017 at 06:06:48AM -0800, Matthew Wilcox wrote:
> > > Unfortunately for you, I don't find arguments along the lines of
> > > "lockdep will save us" at all convincing. lockdep already throws
> > > too many false
On Wed, Dec 06, 2017 at 06:06:48AM -0800, Matthew Wilcox wrote:
> > Unfortunately for you, I don't find arguments along the lines of
> > "lockdep will save us" at all convincing. lockdep already throws
> > too many false positives to be useful as a tool that reliably and
> > accurately points out
On Wed, Dec 06, 2017 at 06:06:48AM -0800, Matthew Wilcox wrote:
> On Wed, Dec 06, 2017 at 07:44:04PM +1100, Dave Chinner wrote:
> > On Tue, Dec 05, 2017 at 08:45:49PM -0800, Matthew Wilcox wrote:
> > > That said, using xa_cmpxchg() in the dquot code looked like the right
> > > thing to do? Since
On Wed, Dec 06, 2017 at 07:44:04PM +1100, Dave Chinner wrote:
> On Tue, Dec 05, 2017 at 08:45:49PM -0800, Matthew Wilcox wrote:
> > That said, using xa_cmpxchg() in the dquot code looked like the right
> > thing to do? Since we'd dropped the qi mutex and the ILOCK, it looks
> > entirely
On Tue, Dec 05, 2017 at 08:45:49PM -0800, Matthew Wilcox wrote:
> On Wed, Dec 06, 2017 at 02:14:56PM +1100, Dave Chinner wrote:
> > > The other conversions use the normal API instead of the advanced API, so
> > > all of this gets hidden away. For example, the inode cache does this:
> >
> > Ah,
On Tue, Dec 05, 2017 at 08:45:49PM -0800, Matthew Wilcox wrote:
> The dquot code is just going to have to live with the fact that there's
> additional locking going on that it doesn't need. I'm open to getting
> rid of the irqsafety, but we can't give up the spinlock protection
> without giving
On Wed, Dec 06, 2017 at 02:14:56PM +1100, Dave Chinner wrote:
> > The other conversions use the normal API instead of the advanced API, so
> > all of this gets hidden away. For example, the inode cache does this:
>
> Ah, OK, that's not obvious from the code changes. :/
Yeah, it's a lot easier
On Tue, Dec 05, 2017 at 06:02:08PM -0800, Matthew Wilcox wrote:
> On Wed, Dec 06, 2017 at 12:36:48PM +1100, Dave Chinner wrote:
> > > - if (radix_tree_preload(GFP_NOFS))
> > > - return -ENOMEM;
> > > -
> > > INIT_LIST_HEAD(>list_node);
> > > elem->key = key;
> > >
> > > -
On Wed, Dec 06, 2017 at 12:36:48PM +1100, Dave Chinner wrote:
> > - if (radix_tree_preload(GFP_NOFS))
> > - return -ENOMEM;
> > -
> > INIT_LIST_HEAD(>list_node);
> > elem->key = key;
> >
> > - spin_lock(>lock);
> > - error = radix_tree_insert(>store, key, elem);
> > -
On Tue, Dec 05, 2017 at 04:41:58PM -0800, Matthew Wilcox wrote:
> From: Matthew Wilcox
>
> This eliminates a call to radix_tree_preload().
.
> void
> @@ -431,24 +424,24 @@ xfs_mru_cache_insert(
> unsigned long key,
> struct xfs_mru_cache_elem
From: Matthew Wilcox
This eliminates a call to radix_tree_preload().
Signed-off-by: Matthew Wilcox
---
fs/xfs/xfs_mru_cache.c | 72 +++---
1 file changed, 33 insertions(+), 39 deletions(-)
diff --git
29 matches
Mail list logo