On Thu, Jan 16, 2025 at 11:55:34AM +0300, Nazir Bilal Yavuz wrote:
> I checked clang 4 as well on the link you sent and it also fixes the
> warning there.
Confirmed here, so done this way.
--
Michael
signature.asc
Description: PGP signature
Hi,
On Thu, 16 Jan 2025 at 10:12, Bertrand Drouvot
wrote:
>
> Hi,
>
> On Thu, Jan 16, 2025 at 12:47:17AM -0500, Tom Lane wrote:
> > Michael Paquier writes:
> > > Not completely sure about the number of parenthesis, but I hope that
> > > this should be enough (extra set around io_op):
> > > +#def
On Thu, Jan 16, 2025 at 07:12:34AM +, Bertrand Drouvot wrote:
> From what I can see, the above proposal does (at least) silent the warning
> here (clang 5.0.1 and same as demoiselle): https://godbolt.org/z/cGosfzGne (we
> can see the warning by using the current define and that the warning is g
Hi,
On Thu, Jan 16, 2025 at 12:47:17AM -0500, Tom Lane wrote:
> Michael Paquier writes:
> > Not completely sure about the number of parenthesis, but I hope that
> > this should be enough (extra set around io_op):
> > +#define pgstat_is_ioop_tracked_in_bytes(io_op) \
> > + (((unsigned int) (io_o
Michael Paquier writes:
> Not completely sure about the number of parenthesis, but I hope that
> this should be enough (extra set around io_op):
> +#define pgstat_is_ioop_tracked_in_bytes(io_op) \
> + (((unsigned int) (io_op)) < IOOP_NUM_TYPES && \
> + ((unsigned int) (io_op)) >= IOOP_EXT
On Thu, Jan 16, 2025 at 12:18:38AM -0500, Tom Lane wrote:
> However, the macro does provide a convenient place to hang the
> warning comment about keeping it sync'd with the enum.
> Personally I'd keep the macro but move it to pgstat.h, close
> to the enum declaration, so that there's more than eps
Michael Paquier writes:
> Just for an assert, I would just remove the macro rather than have an
> inline function.
Oh, I'd not noticed that there is only one caller.
However, the macro does provide a convenient place to hang the
warning comment about keeping it sync'd with the enum.
Personally I
On Wed, Jan 15, 2025 at 11:34:14PM -0500, Tom Lane wrote:
> Michael Paquier writes:
> > I cannot reproduce that, perhaps I'm just missing something with these
> > switches. Do you think that a cast would cool things? Please see the
> > attached for the idea.
>
> There are only three animals sho
Michael Paquier writes:
> I cannot reproduce that, perhaps I'm just missing something with these
> switches. Do you think that a cast would cool things? Please see the
> attached for the idea.
There are only three animals showing this warning (ayu, batfish,
demoiselle) so it likely requires par
On Wed, Jan 15, 2025 at 12:19:03PM -0500, Tom Lane wrote:
> I don't see a reasonable way to alter that check to suppress this;
> for instance, "(io_op) <= IOOP_WRITE" would probably still draw the
> same warning. I think most likely we have to remove that check, ie
>
> #define pgstat_is_ioop_tra
Nazir Bilal Yavuz writes:
> On Tue, 14 Jan 2025 at 06:18, Michael Paquier wrote:
>> And I've somewhat managed to fat-finger the business with
>> pgstat_count_io_op() with an incorrect rebase. Will remove in a
>> minute..
> Thank you!
Commit f92c854cf has caused some of the buildfarm members to
Hi,
On Tue, 14 Jan 2025 at 06:18, Michael Paquier wrote:
>
> On Fri, Jan 10, 2025 at 08:23:46AM +, Bertrand Drouvot wrote:
> > On Fri, Jan 10, 2025 at 10:15:52AM +0300, Nazir Bilal Yavuz wrote:
> >> But I agree that having a macro has more benefits,
> >> also there already is a check for the
On Fri, Jan 10, 2025 at 08:23:46AM +, Bertrand Drouvot wrote:
> On Fri, Jan 10, 2025 at 10:15:52AM +0300, Nazir Bilal Yavuz wrote:
>> But I agree that having a macro has more benefits,
>> also there already is a check for the 'io_op < IOOP_NUM_TYPES' in the
>> pgstat_count_io_op() function.
>
Hi,
On Fri, Jan 10, 2025 at 10:15:52AM +0300, Nazir Bilal Yavuz wrote:
> Hi,
>
> On Fri, 10 Jan 2025 at 04:47, Michael Paquier wrote:
> >
> > On Thu, Jan 09, 2025 at 03:30:37PM +, Bertrand Drouvot wrote:
> > > We first use an Assert in is_ioop_tracked_in_bytes() and then we return
> > > a va
Hi,
On Fri, 10 Jan 2025 at 04:47, Michael Paquier wrote:
>
> On Thu, Jan 09, 2025 at 03:30:37PM +, Bertrand Drouvot wrote:
> > We first use an Assert in is_ioop_tracked_in_bytes() and then we return
> > a value "just" to check another Assert. I wonder if it wouldn't make more
> > sense
> > t
On Thu, Jan 09, 2025 at 03:30:37PM +, Bertrand Drouvot wrote:
> We first use an Assert in is_ioop_tracked_in_bytes() and then we return
> a value "just" to check another Assert. I wonder if it wouldn't make more
> sense
> to get rid of this function and use a macro instead, something like?
>
Hi,
On Thu, Jan 09, 2025 at 02:20:16PM +0300, Nazir Bilal Yavuz wrote:
> Hi,
>
> On Thu, 9 Jan 2025 at 11:11, Michael Paquier wrote:
> >
> > On Thu, Jan 09, 2025 at 10:15:20AM +0300, Nazir Bilal Yavuz wrote:
> > > I am a bit confused, are you suggesting these two alternatives:
> > > 1- Making pg
Hi,
On Thu, 9 Jan 2025 at 11:11, Michael Paquier wrote:
>
> On Thu, Jan 09, 2025 at 10:15:20AM +0300, Nazir Bilal Yavuz wrote:
> > I am a bit confused, are you suggesting these two alternatives:
> > 1- Making pgstat_count_io_op_n() static and continuing to use
> > pgstat_count_io_op() as it is.
>
On Thu, Jan 09, 2025 at 10:15:20AM +0300, Nazir Bilal Yavuz wrote:
> I am a bit confused, are you suggesting these two alternatives:
> 1- Making pgstat_count_io_op_n() static and continuing to use
> pgstat_count_io_op() as it is.
> 2- Removing pgstat_count_io_op() and instead using
> pgstat_count_i
Hi,
On Thu, 9 Jan 2025 at 10:15, Nazir Bilal Yavuz wrote:
>
> On Thu, 9 Jan 2025 at 05:59, Michael Paquier wrote:
> >
> >
> > +static inline bool
> > +is_ioop_tracked_in_bytes(IOOp io_op)
> > +{
> > +Assert((unsigned int) io_op < IOOP_NUM_TYPES);
> > +return io_op >= IOOP_EXTEND;
> > +}
Hi,
Thanks for the review!
On Thu, 9 Jan 2025 at 05:59, Michael Paquier wrote:
>
> On Thu, Dec 26, 2024 at 02:41:26PM +0300, Nazir Bilal Yavuz wrote:
> > Thanks! v4 is attached. I quickly tested the pg_stat_get_backend_io()
> > function and it seems it is working.
>
> Thanks a lot for the rebase
On Thu, Dec 26, 2024 at 02:41:26PM +0300, Nazir Bilal Yavuz wrote:
> Thanks! v4 is attached. I quickly tested the pg_stat_get_backend_io()
> function and it seems it is working.
Thanks a lot for the rebased version. This looks pretty solid. Here
are some comments.
void
-pgstat_count_io_op(IOOb
Hi,
On Tue, 24 Dec 2024 at 09:13, Michael Paquier wrote:
>
> On Fri, Dec 06, 2024 at 12:41:55PM +0300, Nazir Bilal Yavuz wrote:
> > Thanks! I think 'track' is a better word in this context. I used
> > 'tracked in ...', as it sounded more correct to me (I hope it is).
>
> Splitting op_bytes into t
On Fri, Dec 06, 2024 at 12:41:55PM +0300, Nazir Bilal Yavuz wrote:
> Thanks! I think 'track' is a better word in this context. I used
> 'tracked in ...', as it sounded more correct to me (I hope it is).
Splitting op_bytes into three fields sounds like a good idea. Count
me in regarding the concep
Hi,
On Thu, 5 Dec 2024 at 12:13, Bertrand Drouvot
wrote:
>
> Hi,
>
> On Wed, Dec 04, 2024 at 02:49:11PM +0300, Nazir Bilal Yavuz wrote:
> > On Thu, 28 Nov 2024 at 16:39, Bertrand Drouvot
> > wrote:
> > >
> > You are right, no need to have this check; it can not be less than 0.
> > I completely r
Hi,
On Wed, Dec 04, 2024 at 02:49:11PM +0300, Nazir Bilal Yavuz wrote:
> On Thu, 28 Nov 2024 at 16:39, Bertrand Drouvot
> wrote:
> >
> You are right, no need to have this check; it can not be less than 0.
> I completely removed the function now.
Thanks! Yeah I think that makes sense.
> > What
Hi,
Thanks for looking into this!
On Thu, 28 Nov 2024 at 16:39, Bertrand Drouvot
wrote:
>
> Hi,
>
> On Wed, Nov 27, 2024 at 11:08:01AM -0500, Melanie Plageman wrote:
> > On Wed, Sep 11, 2024 at 7:19 AM Nazir Bilal Yavuz
> > wrote:
> > >
> > > Currently, in the pg_stat_io view, IOs are counted
Hi,
On Wed, Nov 27, 2024 at 11:08:01AM -0500, Melanie Plageman wrote:
> On Wed, Sep 11, 2024 at 7:19 AM Nazir Bilal Yavuz wrote:
> >
> > Currently, in the pg_stat_io view, IOs are counted as blocks. However,
> > there are two issues with this approach:
> >
> > 1- The actual number of IO requests
On Wed, Sep 11, 2024 at 7:19 AM Nazir Bilal Yavuz wrote:
>
> Currently, in the pg_stat_io view, IOs are counted as blocks. However, there
> are two issues with this approach:
>
> 1- The actual number of IO requests to the kernel is lower because IO
> requests can be merged before sending the fin
29 matches
Mail list logo