Messages in this thread | | | Date | Thu, 20 Nov 2014 16:14:29 +0200 | From | Boaz Harrosh <> | Subject | Re: [PATCH vfs 2/2] {block|char}_dev: remove inode->i_devices |
| |
On 11/20/2014 03:11 PM, Tejun Heo wrote: > Hello, Boaz. > <> > W/ preloading, one way to do it is, > > if (preload()) > handle -ENOMEM; > lock; > error = insert(); > if (error) > handle error which can't be -ENOMEM; > unlock; > preload_end(); >
I like this one, cause of the place I come from. Where in a cluster you want the local fails as early as possible before you start to commit remotely, and need to undo on errors.
And I can see the real flow of things
> Another way is > > preload(); // can't fail > lock; > error = insert(); > if (error) > handle error;' > unlock; > preload_end(); > > Both ways have pros and cons. The latter seems to lead to simpler > code in general. Not always, but the overall. >
I still like the over all simplicity of the above pattern to this behind the seen complexity hidden away under the carpet.
But I guess that is just me. That is your call sir.
I do see your point though.
<> > > And that's why the pattern usually leads to simpler code - it doesn't > create a new failure point. >
Again a matter of taste. I like the extra ENOMEM failure point before I started to commit to any state changes, lock grabbing and unrolling in case of errors.
But I see your points as well. For what it is worth I have reviewed your code and did not find any faults in it. It looks like sound code.
Thanks Boaz
| |